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

fix preference loading for non string value #41294

Merged
merged 1 commit into from
Jun 21, 2021

Conversation

Roger-luo
Copy link
Contributor

@Roger-luo Roger-luo commented Jun 21, 2021

this fix JuliaPackaging/Preferences.jl#16 kinda revert the type annotation added in #38285 since we don't know the type of a value IIUC unless we want to restrict the preference value to be String only.

I'm not sure where to add a test for this, or get_preferences_hash wasn't tested at all?

guess cc: @staticfloat

@KristofferC
Copy link
Sponsor Member

Would indeed be good to have some docs / tests for this (ref #38459). Otherwise, it is hard to not introduce regressions or break features as happened here.

@Roger-luo
Copy link
Contributor Author

Roger-luo commented Jun 21, 2021

I can add some docs + tests in this PR if preferred, but very likely to need some help from @staticfloat (or could do that in a seperate PR which I guess would be faster to get this PR merged?)


PS. @KristofferC can we backport this if it's not too late?

@KristofferC
Copy link
Sponsor Member

Yes, it seems to be a standard bugfix.

@KristofferC KristofferC added backport 1.6 Change should be backported to release-1.6 backport 1.7 kind:bugfix This change fixes an existing bug labels Jun 21, 2021
@staticfloat
Copy link
Sponsor Member

Yep, this is definitely a regression; we absolutely want to support dicts. Roger, adding docs and tests in a separate PR would be great, please ping me when you do so and I'll provide guidance/feedback.

@staticfloat staticfloat merged commit 7553ca1 into JuliaLang:master Jun 21, 2021
KristofferC pushed a commit that referenced this pull request Jun 26, 2021
(cherry picked from commit 7553ca1)
KristofferC pushed a commit that referenced this pull request Jun 29, 2021
(cherry picked from commit 7553ca1)
@KristofferC KristofferC mentioned this pull request Jun 29, 2021
45 tasks
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
@KristofferC KristofferC removed backport 1.7 backport 1.6 Change should be backported to release-1.6 labels Jul 7, 2021
jessymilare pushed a commit to jessymilare/julia that referenced this pull request Sep 11, 2021
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
(cherry picked from commit 7553ca1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fail to load value that is a Dict during precompile
3 participants