Recompile on profile rustflag changes#11121
Recompile on profile rustflag changes#11121Hezuikn wants to merge 3 commits intorust-lang:masterfrom
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
| /// "dev" which are essentially the same. | ||
| #[derivative(Hash, PartialEq)] | ||
| pub struct Profile { | ||
| #[derivative(Hash = "ignore", PartialEq = "ignore")] |
There was a problem hiding this comment.
Could you re-do the commits so that one commit is the refactor with the other commit having the fix? And describe the problem and the fix in the PR?
This is putting the burden on the reviewer to figure out what is happening with this and the team is already strapped for availability.
| /// Profile settings used to determine which compiler flags to use for a | ||
| /// target. | ||
| #[derive(Clone, Eq, PartialOrd, Ord, serde::Serialize)] | ||
| #[derive(Clone, Eq, PartialOrd, Ord, serde::Serialize, derivative::Derivative)] |
There was a problem hiding this comment.
When adding a dependency, please expand on why it is helpful and why it is worth taking on.
In particular, this is adding a dependency in place of hand-implementing one Hash and one PartialEq. That seems like a fairly small scope that I would expect the dependency to be pulling a lot of weight.
There was a problem hiding this comment.
i don't feel code like that should be allowed to exist
There was a problem hiding this comment.
i don't feel code like that should be allowed to exist
Please expand.
Communication is critical in open source, especially when maintains have limited availability. So far the communication has been pretty minimal between this comment, this PR, and #11120.
I can make my own guesses and some of them might be quite valid but that is a one sided conversation with myself. I'm wanting to understand your intentions and reasoning.
Hezuikn
force-pushed
the
profile-rustflags-fix
branch
from
681775b to
3b3f447
Compare
Hezuikn
force-pushed
the
profile-rustflags-fix
branch
from
3b3f447 to
d8449e9
Compare
weihanglo
added
S-waiting-on-author
|
Ping @Hezuikn. Just checking in to see if you are still interested in working on this. As epage said, a good conversation often leads to a better context of understanding to the solution. If you need helps, comment here and we could take a look. |
|
yes! i am |
|
☔ The latest upstream changes (presumably #11719) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Thank you for your interest in this. However, I'm going to close due to inactivity. Feel free to open a new one if you have a chance to work on it again. |
6 participants
turns out rustflags effect compilation 4head
Fixes #11120