-
Notifications
You must be signed in to change notification settings - Fork 168
0.6.2 #187
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
0.6.2 #187
Conversation
r? @therealprof (rust_highfive has picked a reviewer for you, use r? to override) |
build.rs
Outdated
println!("cargo:rustc-cfg=cortex_m"); | ||
println!("cargo:rustc-cfg=armv7m"); | ||
} else if target.starts_with("thumbv7em-") { | ||
} else if target.starts_with("thumbv7m-") || target.starts_with("thumbv7em-") { |
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.
I'd rather we do not make this change. If the compiler happens to add special support for thumbv7em
which is more than just the DSP extension, e.g. saturated instructions and FPU support, then we do not need to update this crate and all dependencies again.
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.
This is not a functional change, it just silences a Clippy lint which otherwise fails CI on nightly. Unless I misunderstand what you're saying?
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.
I'm refererring to the comment new comment below. This code changes the flags passed to rustc based on the architecture we're compiling for and you're saying it does not make sense to pass the armv7em
flag because it's not supported at the moment.
If this fixes a clippy lint (not mentioned in the comment) and has no other drawbacks (the opposite of which is implied by the comment, namely that we'll have to add it back once supported) I'm okay with the change but would like to see those facts recorded in the comment. To me it currently reads as if we're simply removing this because we don't need to have now but will later and then have to add it back.
Even though this cfg isn't currently used, printing it doesn't hurt, and it avoids a Clippy lint
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.
LGTM, thanks.
bors r+
187: 0.6.2 r=therealprof a=jonas-schievink Closes #185 Co-authored-by: Jonas Schievink <[email protected]>
Build succeeded |
... and published. |
187: Allow nightly to fail in Travis r=therealprof a=korken89 With nightly being a bit skiddish, lets allow nightly to fail. Anyone against? @rust-embedded/cortex-m - [x] @adamgreig - [x] @korken89 - [x] @thejpster - [ ] @ithinuel - [x] @therealprof Co-authored-by: Emil Fresk <[email protected]>
Closes #185