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

remove flavors #246

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

remove flavors #246

wants to merge 3 commits into from

Conversation

planthaber
Copy link
Member

No description provided.

@planthaber
Copy link
Member Author

Base automatically changed from remove_flavor_sanity_checks to master November 13, 2024 23:40
@doudou
Copy link
Member

doudou commented Nov 13, 2024

OK. That's where I don't agree. Maybe what you meant by "backward compatibility"

Keep the in_flavor call, just make it a no-op and make sure you issue a warning whenever one calls it. We can't assume that noone will have a in_flavor somewhere that would break all of a sudden.

Also, keep the config variables (ROCK_SELECTED_FLAVOR, ROCK_FLAVOR and ROCK_BRANCH), just force-set them to master if they aren't defined (but keep whatever existing value is in the config if there is one). ROCK_BRANCH is in particular used in rock package sets. Since this PR does not modify source.yml, I'm assuming this just broke ... everything ?

In 10 years when everyone is fed up with the warning, we'll remove the rest.

@planthaber
Copy link
Member Author

This is what i meant with "Do we need deprecation" in the other PR, but anyways, re-added the functions

Since this PR does not modify source.yml, I'm assuming this just broke ... everything ?

Actually I had the var still in the confug.yml from inital checkout. This is why nothing broke when i was testing.

Also, keep the config variables (ROCK_SELECTED_FLAVOR, ROCK_FLAVOR and ROCK_BRANCH), just force-set them to master if they aren't defined (but keep whatever existing value is in the config if there is one).

This could also raise an issue when a workspace that needs the vars is bootstrapped into another location where the vars are not already existing.

Anyways:

  • re-added no-op function with warning
  • sets ROCK_SELECTED_FLAVOR, ROCK_FLAVOR and ROCK_BRANCH if not existent
  • removed $ROCK_BRANCH from source.yml

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.

2 participants