-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove the distinction between --select and --models flags when working with node types #3210
Comments
@joellabes I agree! In all likelihood, I think we should:
There are two things we'll need to sort out before we do this: Trivial: listThe Trickier: test selection
Before we fully switch
This still gets us the compelling syntax we'd want while handling the worst surprises.
I realize that, by saying this, I'm blocking a semantic change behind a meatier functional change—maybe it's not fair to use one as the forcing function for the other—let me know if you disagree! In any case, I think this is something we should seriously consider doing ahead of v1.0. |
I think you've explained the magic jump to me a couple of times in the past, but I get it this time! Thanks.
I'd make a half-hearted argument that flags' meanings (especially short flags) can be anything we want them to be. The strongest argument to back this up is that Side note: realistically, this all only applies to custom data tests, right? Am I right that (for selection purposes) schema tests' True Names are the kwarg portmanteaus we all know and love? If so, then I doubt anyone is typing
glosses over a lot of the complexity. Having those two PLUS selecting a single instance of a schema test would probably be gnarlier to build a mental model upon. For the record, I am on board with the magic modulation! But I don't think it has to block this. With that said, this isn't at all urgent, so if we wanted to put it on the backburner until after #2891 then I'd be fine. Also, agreed that supporting |
Ok, I think all your instincts here are right! We should do this, and we should also do #2891; we should both before v1.0. Let's hold off on it for the moment, not on the backburner per se, but in the spirit of planning a number of nomenclature changes for the minor versions between v0.20 and v1.0. |
|
@joellabes I'd love to get this in for v0.21. Is this still something you'd be interested in contributing? We can offer some helping hands along the way :) |
@jtcohen6 yep let's give it a whirl! |
@gshank I'd really like to prioritize this one and get it in for v0.21. This will go nicely with the more-consistent |
[#3210] Make --models and --select synonyms, except for 'ls'
Describe the feature
It would be less confusing if all nodes were selectable using a single flag, instead of having to switch back and forth.
Describe alternatives you've considered
Getting over it
Additional context
I imagine it would make more sense to settle on
--select and -s
because they're more broadly applicable. The migration might be uncomfortable, but could be a slow-boiled frog in the same vein asconfig-version
:--select
is the default version in the docs etc for new users and-m
users keep going without any issues-m
users get a WARN during compilation-m
is deprecated and no longer respected.Or, just add
-s
as an alias and leave the old ones around forever; they're not really doing any harm!Who will this benefit?
New users in particular (this confused me ~9 months ago), but it also just bit me again 😑.
Are you interested in contributing this feature?
Sure! I'm sure I could work it out by looking at how the
-model
alias is currently handled. Most of the work feels like it would be in the deprecation warningsThe text was updated successfully, but these errors were encountered: