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 warnings for rust 1.80 #5150

Merged
merged 8 commits into from
Jul 26, 2024
Merged

Conversation

gui1117
Copy link
Contributor

@gui1117 gui1117 commented Jul 26, 2024

Fix warnings for rust 1.80

@@ -17,7 +17,6 @@

// Custom inner attributes are unstable, so we need to faky disable the attribute.
// rustfmt still honors the attribute to not format the rustdocs below.
#![cfg_attr(feature = "never", rustfmt::skip)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you read the comment above? :D Not sure this is already fixed in rustfmt?

Copy link
Contributor Author

@gui1117 gui1117 Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed it, but I did run cargo fmt -p sp-runtime-interface the file was unchanged so I thought it wasn't relevant anymore.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI uses a specific nightly for formatting: cargo +nightly-2024-04-14 fmt.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also tested with this version, anyway CI should tell if the formatting of the crate is changing with this PR, in this case I'll revert this change.

@@ -35,6 +35,7 @@ std = [
"sp-arithmetic/std",
"sp-debug-derive/std",
]
runtime-benchmarks = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there code in sp-weights that is guarded by the feature?

Copy link
Contributor Author

@gui1117 gui1117 Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this, we could also remove it IMO:

#[cfg(any(test, feature = "std", feature = "runtime-benchmarks"))]
impl From<u64> for Weight {
fn from(value: u64) -> Self {
Self::from_parts(value, value)
}
}
#[cfg(any(test, feature = "std", feature = "runtime-benchmarks"))]
impl From<(u64, u64)> for Weight {
fn from(value: (u64, u64)) -> Self {
Self::from_parts(value.0, value.1)
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it was never used, let's remove it.

Cargo.toml Show resolved Hide resolved
@paritytech-review-bot paritytech-review-bot bot requested a review from a team July 26, 2024 09:19
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6821864

@paritytech-review-bot paritytech-review-bot bot requested a review from a team July 26, 2024 10:16
@@ -869,7 +869,6 @@ macro_rules! hypothetically_ok {
pub use serde::{Deserialize, Serialize};

#[doc(hidden)]
#[cfg(not(no_std))]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually needed in both std and not std: #5150 (comment)

@@ -401,14 +401,14 @@ where
}
}

#[cfg(any(test, feature = "std", feature = "runtime-benchmarks"))]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better not to add some conditional code for runtime-benchmark here IMO.
This code was never generated, better not to start generating it.

Copy link
Member

@ggwpez ggwpez Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea feel free to delete in that case. I normally generate helpers on the fly if i need to.

@bkchr bkchr added the R0-silent Changes should not be mentioned in any release notes label Jul 26, 2024
@bkchr bkchr enabled auto-merge July 26, 2024 16:27
@bkchr bkchr added this pull request to the merge queue Jul 26, 2024
Merged via the queue into paritytech:master with commit 7250937 Jul 26, 2024
163 of 167 checks passed
@gui1117 gui1117 deleted the gui-fix-warnings branch July 26, 2024 23:03
github-merge-queue bot pushed a commit that referenced this pull request Jul 28, 2024
I made mistake on previous PR
#5150. It disabled all
unexpected cfgs instead of just allowing `substrate_runtime` condition.

In this PR: unexpected cfgs other than `substrate_runtime` are still
checked. and some warnings appear about them
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
I made mistake on previous PR
paritytech#5150. It disabled all
unexpected cfgs instead of just allowing `substrate_runtime` condition.

In this PR: unexpected cfgs other than `substrate_runtime` are still
checked. and some warnings appear about them
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants