- 
                Notifications
    You must be signed in to change notification settings 
- Fork 18
Prefer setting preferences only within the currently active project #37
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
Conversation
Previous to this change, we would search up the environment stack looking for the project that contains the target package, and set the preference in there. This was intended to make it convenient to e.g. set a `Revise` preference while having an application project activated. It turns out that the more common problem is actually that a package wants to set a preference of a sub-package (e.g. `MPI` and `MPIPreferences`) that may not be a top-level dependent of the currently active project. Therefore, we change the behavior of `set_preferences!()` to prefer adding the target package as an `extras` dependency of the currently-active project, but maintain the old behavior behind a keyword argument to `set_preferences!()`.
| Codecov Report
 @@            Coverage Diff             @@
##           master      #37      +/-   ##
==========================================
+ Coverage   76.52%   85.71%   +9.19%     
==========================================
  Files           2        2              
  Lines         115      126      +11     
==========================================
+ Hits           88      108      +20     
+ Misses         27       18       -9     
 📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more | 
| Technically breaking? | 
| uuid = "21216c6a-2e73-6563-6e65-726566657250" | ||
| authors = ["Elliot Saba <elliot.saba@juliacomputing.com>", "contributors"] | ||
| version = "1.2.5" | ||
| version = "1.3.0" | 
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.
| version = "1.3.0" | |
| version = "2.0.0-DEV" | 
Unless you know you don't plan on making any other breaking changes anytime soon, in which case you could just set it to 2.0.0 and cut a new release after this PR is merged.
| I'm not really sure this counts as breaking.... I don't think anyone is relying on this implementation detail, and it wasn't even documented. @vchuravy what do you think? | 
| Hmmm, if the original behavior wasn't documented, then it's probably not breaking to change the behavior. | 
| One could even argue that it's a bug fix since we have in the docs: 
 Which didn't work. We would wipe the preferences higher up in the chain instead. | 
Previous to this change, we would search up the environment stack
looking for the project that contains the target package, and set the
preference in there. This was intended to make it convenient to e.g.
set a
Revisepreference while having an application project activated.It turns out that the more common problem is actually that a package
wants to set a preference of a sub-package (e.g.
MPIandMPIPreferences) that may not be a top-level dependent of the currentlyactive project.
Therefore, we change the behavior of
set_preferences!()to preferadding the target package as an
extrasdependency of thecurrently-active project, but maintain the old behavior behind a keyword
argument to
set_preferences!().