Skip to content
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

Exclude targets in plugin subrepos from plz query changes #2722

Conversation

chrisnovakovic
Copy link
Contributor

Changing a plugin_repo() target causes all of the plugin subrepo's child targets to be outputted by plz query changes. This is problematic because it isn't necessarily desirable (or even possible, depending on the environment) to build every single target in the plugin subrepo.

Exclude all plugin subrepo child targets from the output of plz query changes by default, and add an --include_plugins option to revert to the previous behaviour if desired.

Changing a `plugin_repo()` target causes all of the plugin subrepo's
child targets to be outputted by `plz query changes`. This is
problematic because it isn't necessarily desirable (or even possible,
depending on the environment) to build every single target in the plugin
subrepo.

Exclude all plugin subrepo child targets from the output of `plz query
changes` by default, and add an `--include_plugins` option to revert to
the previous behaviour if desired.
@@ -416,6 +416,7 @@ var opts struct {
Changes struct {
Since string `short:"s" long:"since" default:"origin/master" description:"Revision to compare against"`
IncludeDependees string `long:"include_dependees" default:"none" choice:"none" choice:"direct" choice:"transitive" description:"Deprecated: use level 1 for direct and -1 for transitive. Include direct or transitive dependees of changed targets."`
IncludePlugins bool `long:"include_plugins" description:"Include changed targets that belong to plugin subrepos."`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should just be all subrepos? I feel like I would probably want the same behaviour most of the time from non-plugin repos as well (e.g. if you pulled in something like absl or googletest as a subrepo, I don't think I want to run all their tests on upgrade)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could see a case for wanting to detect changes in targets in subrepos created by github_repo build files etc. though. Maybe there should instead be a hide_changes parameter to subrepo() which controls this behaviour? It could be set to true in the plugin_repo() def, and left up to the user for other cases.

@chrisnovakovic
Copy link
Contributor Author

There's consensus that the right way to do this is for all subrepos, not just plugin subrepos, which I've implemented in #2723 - thanks everyone for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants