Deserialize Msrv directly in Conf#11683
Conversation
|
r? @Manishearth (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
Alexendoo
force-pushed
the
msrv-config
branch
from
e291bb9 to
f36f59c
Compare
|
@bors r+ |
Deserialize `Msrv` directly in `Conf` Gives the error a span pointing to the invalid config value Also puts `Conf` itself in the `OnceLock` rather than just the `Msrv` for [the `register_late_mod_pass` work](rust-lang/rust#116731) since it will be used from two different callbacks changelog: none
|
💔 Test failed - checks-action_test |
Alexendoo
force-pushed
the
msrv-config
branch
from
f36f59c to
1528c1d
Compare
| The minimum rust version that the project supports | ||
|
|
||
| **Default Value:** `None` (`Option<String>`) | ||
| **Default Value:** `Msrv { stack: [] }` (`crate::Msrv`) |
There was a problem hiding this comment.
Huh, why is the default value of MSRV an empty stack? 🤔
EDIT: Ah I see why now. But this might be confusing to the user. I'm not sure what to do about this though, so I don't want to block this PR on this. I really like the code improvements and the pointing to the value.
There was a problem hiding this comment.
Ah damn I didn't think about that, I think we need to change that in general since other config values also show internal type names which isn't helpful for public documentation
There was a problem hiding this comment.
Right, I don't think this has to be done in this PR though. So feel free to r=me here now. But if you feel like doing it in this PR, I won't stop you 🙃
|
@bors r=Manishearth,flip1995 I'll think about the config thing separately, I think it would need website changes as well |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
5 participants
Gives the error a span pointing to the invalid config value
Also puts
Confitself in theOnceLockrather than just theMsrvfor theregister_late_mod_passwork since it will be used from two different callbackschangelog: none