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

Update Rust version #473

Conversation

AndreiEres
Copy link
Contributor

@AndreiEres AndreiEres commented Oct 8, 2024

Fixes #470

  • Fixed formatting
  • Doc comments were split making first doc comment not too long
  • Fixed lists in doc comments
  • Unreachable patterns silenced with #[allow(unreachable_patterns)] (controversial)
  • time updated to 0.3.36
  • Does not require a CHANGELOG entry

@AndreiEres
Copy link
Contributor Author

@ggwpez, can you tell me if it's acceptable for "Check Migrations" to fail here?

@AndreiEres AndreiEres marked this pull request as ready for review October 15, 2024 12:39
@ggwpez
Copy link
Member

ggwpez commented Oct 17, 2024

@ggwpez, can you tell me if it's acceptable for "Check Migrations" to fail here?

Yea am fixing it here: #463

@@ -18,6 +18,7 @@

pub use pallet_custom_origins::*;

#[allow(unreachable_patterns)]
Copy link
Member

@ggwpez ggwpez Oct 17, 2024

Choose a reason for hiding this comment

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

What warning is this suppressing? I removed them locally and just running clippy seems fine.
Otherwise please open a bug report in the SDK, the FRAME macros should not produce warnings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's try to remove it and look at CI

Copy link
Contributor

Choose a reason for hiding this comment

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

Otherwise please open a bug report in the SDK, the FRAME macros should not produce warnings.

Wasn't there a fix for something similar in the SDK pr that upgraded the rust version?

Copy link
Contributor

Choose a reason for hiding this comment

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

paritytech/polkadot-sdk#5697 okay maybe it wasn't fixed. I checked the pr again.

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Looks good, just the one Q about the FRAME macro warning.

@github-actions github-actions bot requested a review from ggwpez October 17, 2024 17:07
Copy link

Review required! Latest push from author must always be reviewed

@@ -19,6 +19,7 @@
use super::ranks;
pub use pallet_origins::*;

#[allow(unreachable_patterns)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to have an issue to remove this when we have upgraded to the polkadot-sdk version that fixes this.

This reverts commit 7530862.
@bkchr bkchr enabled auto-merge (squash) October 17, 2024 19:30
@bkchr bkchr disabled auto-merge October 17, 2024 19:31
@bkchr
Copy link
Contributor

bkchr commented Oct 17, 2024

/merge

@fellowship-merge-bot
Copy link
Contributor

Enabled auto-merge in Pull Request

Available commands
  • /merge: Enables auto-merge for Pull Request
  • /merge cancel: Cancels auto-merge for Pull Request
  • /merge help: Shows this menu

For more information see the documentation

@fellowship-merge-bot fellowship-merge-bot bot enabled auto-merge (squash) October 17, 2024 19:53
@fellowship-merge-bot fellowship-merge-bot bot merged commit 123f8f4 into polkadot-fellows:main Oct 17, 2024
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Rust version
3 participants