#89 #105
#89 #105
Conversation
There was a problem hiding this comment.
Thanks for the PR. It's not a bad start. In addition to the comments in the PR itself, a few additional things:
- Please provide a more descriptive title to your PR so others can understand what this is about
- Please add a DCO signoff to your commits, per the developers instructions
- This is missing the code loading a user-provided system CTO
- Other commands should be fixed similarly (at least
validateand maybegetas well) - The tests will have to be fixed to reflect your changes (in the
packages/concero-cli/testdirectory)
| @@ -20,13 +20,18 @@ const Commands = require('./lib/commands'); | |||
|
|
|||
| require('yargs') | |||
| .scriptName('concerto') | |||
| .usage('$0 <cmd> [args]') | |||
| .usage('$0 <cmd> [manager] [args]') | |||
There was a problem hiding this comment.
This change to usage is inconsistent with the changes below which use a --ctoSystem option as part of the arguments.
| yargs.option('ctoSystem', { | ||
| describe: 'system model to be used', | ||
| type: 'string' | ||
| default: 'org.accordproject.base.cto' |
There was a problem hiding this comment.
I'm unclear how the default can be a namespace, I was expecting it to be consistent with the option which should pass a file (or maybe a URL or either?) in a way similar to how other CTO files are passed on the command line.
Without a default, the value will be null which might be sufficient to decide whether a system model has been passed or not.
|
Hi @vipulbhj Any progress on this? Any other help you might need? We've had a couple of requests for this fix 🙂 |
|
Hey, sorry for being unresponsive. Its a religion week in india right now celebrate at country scale, so I been a little occupied. I would love to get all the help I can for sure. Thank you so much :) and I be getting back to constant communications in a day or two at max |
|
@vipulbhj I need to push the timeline for this forward since a user is waiting for a fix. I've rebased your change to https://github.com/accordproject/concerto/tree/js-vi-fix-issue-89 which includes your initial code, and I'll be working in this. This should preserve your commit for posterity (and #hacktoberfest). I'm closing this PR and will open a new one for #89 |
Issue #89
Some first attempt changes submitted for review
Changes
Added command option for system and used default model system as a first step
Related Issues