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

Configuration for a builder should not enable that builder #2191

Open
natebosch opened this issue Apr 17, 2019 · 5 comments
Open

Configuration for a builder should not enable that builder #2191

natebosch opened this issue Apr 17, 2019 · 5 comments
Labels
next-breaking-release Issues that are worth doing but need to wait for a breaking version bump

Comments

@natebosch
Copy link
Member

It felt redundant at the time to force enabled: true to turn on a builder, but it turns out that this magic cause some nasty stuff - builders like build_web_compilers:entrypoint start getting turned on for packages other than the root just because someone configured a generate_for or dart2js_args or something.

Changing this would be breaking and it's hard to know how bad the fallout would be.

There isn't currently the capability to express "configure a builder, but only if it would have run anyway."

@natebosch
Copy link
Member Author

In theory we could call this a breaking change on build_config and packages that are doing the "right" think and have a dependency on that package whenever they use build.yaml would be safe - but in practice very few packages are likely doing that.

@jakemac53
Copy link
Contributor

jakemac53 commented Apr 17, 2019

I think step 1 here as we discussed offline is to start complaining if people have a build.yaml but no direct build_config dependency.

@natebosch
Copy link
Member Author

And step 0 is make that dependency less heavy. Blocked by #2192

@jakemac53 jakemac53 added the next-breaking-release Issues that are worth doing but need to wait for a breaking version bump label May 25, 2021
@jakemac53
Copy link
Contributor

Step 0 looks unblocked now 🤣 . Do we want to push on this? Close it?

@natebosch
Copy link
Member Author

I think this one is valid as a long-term backlog "next-breaking-release" issue.

I don't think it is worth a breaking change to solve only this issue. If some other use case for a breaking change in the build.yaml format comes up though, I do think it would be worth changing these semantics at the same time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next-breaking-release Issues that are worth doing but need to wait for a breaking version bump
Projects
None yet
Development

No branches or pull requests

2 participants