Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
🌱 Plugin resolution enhancement proposal #1942
🌱 Plugin resolution enhancement proposal #1942
Changes from all commits
a6042c6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of the advantages implementation-wise of this new approach is that
--project-version
stops being a global variable for all commands that need to be special cased. It will just be a local variable of theInit
command.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There will always be a default project version. If no default plugins support the default project version, then an error should be returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea here is, in case they provide a project version, make decissions knowing it, but in case it wasn't provide, just make decissions as if it was unknown and try to resolve it later.
Imagine I have the following plugins resolved:
If the user provides a
--plugins=go/v3,declarative
flag, we would resolve to the last two plugins, and the only supported project version by both of them is3
so we can implicitly resolve the project version.If the user provides a
--plugins=go/v2,declarative
flag, we would resolve to the first and last plugin, and two different project versions are supported. If any of those is the default project version configured for the CLI, that will be used.So we will have 3 ways of resolving the project version:
--project-version
flag or having an already present config file (version
field).In case none of the three above cases applies, an error ambiguous-project-version will be returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if two default plugins support the same project version? Which will be chosen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this doesn't seem to be a problem if default plugins are forced to have a disjoint set of supported project versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, I may want to specify a plugin for a particular project version, not just the project versions it supports. These changes take my ability to do that away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This proposal takes into account that we will support plugin chains. Having multiple plugins set as default means that it is a default chain. This proposal removes the ability to provide different plugin/plugin-chains for each project version, but I guess we could try to keep this behavior.