DM-21889: Avoid option reuse in pipetask #30
Conversation
andy-slac
force-pushed
the
tickets/DM-21889
branch
from
00a9a2f
to
63c6fa2
Compare
There was a problem hiding this comment.
Changes look great. My only comment is an apology - I think @natelust's work on DM-21421 may actually remove some of the options you had to change, and I should have made sure you were more aware of that when working on that ticket (with Pipelines now at least usually defined in YAML files, the options for specifying tasks to run to pipetask are now more about modifying Pipelines rather than creating them from scratch). That said I still think it totally makes sense to go ahead and merge this and let Nate rebase on top of your changes; I don't think that will be too tricky, and I given that this is basically done, I think that's the right order. You may want to coordinate with him on any interface-change announcements, though, as he'll be changing some of the same ones further shortly after you do.
|
If there is another disruptive interface change then I think it makes sense to do it in one announcement rather than few close ones. @natelust, how close are you to finishing your fixes? Do you want me to delay merging this request into master so that both our merges will be close together? If you need to rebase you can use my branch and then rebase to master again one my branch merges. |
|
My changes should soon be ready to review. The scope of this work keeps creeping a bit. After today I hope to have everything in the state it will be in. I think it might be a good idea to sit down and talk about the changes that are coming. It would probably be best if you @TallJimbo and I were all there. |
andy-slac
force-pushed
the
tickets/DM-21889
branch
from
4fc9003
to
814204b
Compare
Single-letter options are made unique: - --timeout option uses long option - same for --delete - same for --order-pipeline
This commit appears on DM-21889 branch even though the ticket for it is DM-21890, this is to minimize the number of incompatible merges. All global/top-level options have been removed and copied to each sub-command (where they make sense). I have also refactored the code that makes options into a number of separate methods, this makes it cleaner and should also prepare us for the next step of refactoring. All unit tests were updated for new command line interface.
andy-slac
force-pushed
the
tickets/DM-21889
branch
from
814204b
to
c36ae7c
Compare
Single-letter options are made unique:
listsubcommand only uses long options now-p pipeline.pickleis replaced with-r pipeline.pickle