Skip to content

fmt: Use style_edition = 2021#3432

Merged
apoelstra merged 3 commits intorust-bitcoin:masterfrom
tcharding:10-01-fmt
Oct 11, 2024
Merged

fmt: Use style_edition = 2021#3432
apoelstra merged 3 commits intorust-bitcoin:masterfrom
tcharding:10-01-fmt

Conversation

@tcharding
Copy link
Copy Markdown
Member

@tcharding tcharding commented Oct 1, 2024 •
edited
Loading

rustfmt is emitting:

Warning: the version option is deprecated. Use style_edition instead.

As suggested add a config option and set it to 2021.

  • Patch 1: Manually configure rustfmt to skip some code
  • Patch 2: Run the formmater with current configuration
  • Patch 3: Add the new config option (remove old one), introduces no new formatting requirements

@github-actions github-actions bot added C-bitcoin PRs modifying the bitcoin crate C-internals PRs modifying the internals crate C-primitives labels Oct 1, 2024
@tcharding
Copy link
Copy Markdown
Member Author

tcharding commented Oct 1, 2024

I did this just so I could do #3431 without introducing a bucket load of formatting issues but it is going to make rebasing #3408 hard, draft until that goes in.

@tcharding tcharding marked this pull request as draft October 1, 2024 04:00
@tcharding tcharding mentioned this pull request Oct 1, 2024
Merged
@jaoleal
Copy link
Copy Markdown
Contributor

jaoleal commented Oct 9, 2024 •
edited
Loading

Will this be merged ? Im kinda formatting a lot of things just by running just fmt

I need to push to #3433 with correct formatting.

@tcharding
Configure formmater to skip read_compact_size
2c40b4f
@tcharding
Run the formatter
2e4179e 
Run `just fmt`, no other manual changes.
@tcharding
Add rustfmt config option style_edition
323e706 
rustfmt is emitting:

Warning: the version option is deprecated. Use style_edition instead.

As suggested add a config option and set it to 2021.
@tcharding tcharding closed this Oct 9, 2024
@tcharding
Copy link
Copy Markdown
Member Author

tcharding commented Oct 9, 2024

Woops, wrong button

@tcharding tcharding reopened this Oct 9, 2024
@tcharding tcharding marked this pull request as ready for review October 9, 2024 23:43
@tcharding
Copy link
Copy Markdown
Member Author

tcharding commented Oct 9, 2024 •
edited
Loading

Im kinda formatting a lot of things just by running just fmt

@jaoleal quick history, this is the current state of affairs in rust-bitcoin: we had a lot of discussion on how/when/if/if not to use rustfmt (includes a bunch of drama about nightly). The best compromise we could find was

  • Use nightly
  • No CI requirement to be formatted correctly
  • Run a weekly formatting using a bot

This means that sometimes there are a bunch of formatting issues on master, I personally deal with this by running the formatter and commiting the changes locally, then removing the patch before pushing PRs. Usually there are no major conflicts. If there are you can always push up a "run formatter" PR and not wait for the bot.

Its sub-optimal I know but its the best we could come up with to keep everyone kind of happy.

@tcharding
Copy link
Copy Markdown
Member Author

tcharding commented Oct 9, 2024

I'm not sure whats up with the labeler job, perhaps we just kick it again in a few hours.

@tcharding tcharding mentioned this pull request Oct 10, 2024
Closed
@jaoleal
Copy link
Copy Markdown
Contributor

jaoleal commented Oct 10, 2024

Its sub-optimal I know but its the best we could come up with to keep everyone kind of happy.

makes sense, thanks for explaining !

apoelstra
apoelstra approved these changes Oct 11, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 323e706 successfully ran local tests

@apoelstra apoelstra merged commit 0371587 into rust-bitcoin:master Oct 11, 2024
@tcharding tcharding deleted the 10-01-fmt branch October 21, 2024 04:18
tcharding added a commit to tcharding/hex-conservative that referenced this pull request Mar 2, 2026
@tcharding
rustfmt: Fix warnings
7f6c8c3 
The formatter is emitting warnings:

```
cargo +$(cat ./nightly-version) fmt --all
Warning: the `version` option is deprecated. Use `style_edition` instead.
```

Remove `version` and add `stlye_edtion` and set it to 2021.

This is the same as we did in `rust-bitcoin` in 2024, gee wiz

  rust-bitcoin/rust-bitcoin#3432
@tcharding tcharding mentioned this pull request Mar 2, 2026
Merged
apoelstra added a commit to rust-bitcoin/hex-conservative that referenced this pull request Mar 7, 2026
@apoelstra
Merge #207: rustfmt: Fix warnings
eea0f89 
7f6c8c3 rustfmt: Fix warnings (Tobin C. Harding)

Pull request description:

  The formatter is emitting warnings:
  
  ```
  cargo +$(cat ./nightly-version) fmt --all
  Warning: the `version` option is deprecated. Use `style_edition` instead.
  ```
  
  Remove `version` and add `stlye_edtion` and set it to 2021.
  
  This is the same as we did in `rust-bitcoin` in 2024, gee wiz
  
    rust-bitcoin/rust-bitcoin#3432


ACKs for top commit:
  apoelstra:
    ACK 7f6c8c3; successfully ran local tests


Tree-SHA512: 16f00b6ca7212c91ec1a64703ba06bc79fa36d58af88b3beeee3bc08213586e2a657a1190310b8a05ecfd8a109cd7d6595f86d90d4411dd03f8bb3c6e05cbbc9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apoelstra apoelstra apoelstra approved these changes

Assignees

No one assigned

Labels

C-bitcoin PRs modifying the bitcoin crate C-internals PRs modifying the internals crate C-primitives

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tcharding @jaoleal @apoelstra