Conversation
|
Merge along with: rock-core/rock-package_set#269 |
|
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. |
|
This is what i meant with "Do we need deprecation" in the other PR, but anyways, re-added the functions
Actually I had the var still in the confug.yml from inital checkout. This is why nothing broke when i was testing.
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:
|
| def in_flavor(*flavors, &block) | ||
| Rock.flavors.in_flavor(*flavors, &block) | ||
| Autoproj.warn "Flavors system was removed, please remove flavor-related code from your package_sets" | ||
| return false |
There was a problem hiding this comment.
Need to yield for it to be a deprecation. The packages are defined in the block.
| def only_in_flavor(*flavors, &block) | ||
| Rock.flavors.only_in_flavor(*flavors, &block) | ||
| Autoproj.warn "Flavors system was removed, please remove flavor-related code from your package_sets" |
There was a problem hiding this comment.
In this case, only yield if flavors is master.
| def flavor_defined?(flavor_name) | ||
| Rock.flavors.has_flavor?(flavor_name) | ||
| Autoproj.warn "Flavors system was removed, please remove flavor-related code from your package_sets" | ||
| return false |
There was a problem hiding this comment.
true ? flavor_name == "master" ?
| if current_flavor.name != 'master' && Autoproj::PackageSet.respond_to?(:add_source_file) | ||
| Autoproj::PackageSet.add_source_file "source-stable.yml" | ||
| # backward compatibility for the deprecated flavor system | ||
| if !Autoproj.config.has_value_for?('ROCK_SELECTED_FLAVOR') then |
There was a problem hiding this comment.
then is not idiomatic Ruby ... please remove
There was a problem hiding this comment.
If the value exists and is not 'master', I think one should raise an exception. And then unconditionally set ROCK_SELECTED_FLAVOR to master. Same for ROCK_FLAVOR. I would leave the user's choice for ROCK_BRANCH though.
| def package_in_flavor?(pkg, flavor_name) | ||
| Rock.flavors.package_in_flavor?(pkg, flavor_name) | ||
| Autoproj.warn "Flavors system was removed, please remove flavor-related code from your package_sets" | ||
| return false |
There was a problem hiding this comment.
true ? hard to have a sane default, but I think true is better than false
2 participants
No description provided.