Skip to content

Remove Amount::fmt_value_in#3621

Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom
shinghim:2952-remove-fmt-value-in
Nov 14, 2024
Merged

Remove Amount::fmt_value_in#3621
apoelstra merged 1 commit intorust-bitcoin:masterfrom
shinghim:2952-remove-fmt-value-in

Conversation

@shinghim
Copy link
Copy Markdown
Contributor

@shinghim shinghim commented Nov 14, 2024

Saw this mentioned in #3556

Fixes #2952

@github-actions github-actions bot added the C-units PRs modifying the units crate label Nov 14, 2024
@coveralls
Copy link
Copy Markdown

coveralls commented Nov 14, 2024

Pull Request Test Coverage Report for Build 11829236824

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 82.491%
Totals Coverage Status
Change from base Build 11822813449: 0.02%
Covered Lines: 19773
Relevant Lines: 23970
💛 - Coveralls

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 14, 2024

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

@github-actions github-actions bot added the API break This PR requires a version bump for the next release label Nov 14, 2024
@tcharding
Copy link
Copy Markdown
Member

tcharding commented Nov 14, 2024

lol, you managed to put up PRs #3621 and #3612 (note the last two digits), I got mildly confused.

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Nov 14, 2024

I created this discussion to see if we want to merge this: #3622

@shinghim shinghim marked this pull request as ready for review November 14, 2024 04:13
@tcharding tcharding mentioned this pull request Nov 14, 2024
Merged
@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Nov 14, 2024

Yeah, I think it's reasonable to just delete things on master and deprecate them in 0.32.x.

apoelstra
apoelstra approved these changes Nov 14, 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 0348801; successfully ran local tests

@tcharding
Copy link
Copy Markdown
Member

tcharding commented Nov 14, 2024

Yeah, I think it's reasonable to just delete things on master and deprecate them in 0.32.x.

Before anyone goes and deletes all deprecated code, this comment is referring to deprecated code in leaf crates (those soon to be 1.0 e.g, units) not in bitcoin crate. Holla if I'm wrong @apoelstra

tcharding
tcharding approved these changes Nov 14, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 0348801

@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Nov 14, 2024

Yeah, agreed -- we do expect to have more pre-1.0 bitcoin releases so we should keep stuff around in those.

@apoelstra apoelstra merged commit 7f12375 into rust-bitcoin:master Nov 14, 2024
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

@tcharding tcharding tcharding approved these changes

Assignees

No one assigned

Labels

API break This PR requires a version bump for the next release C-units PRs modifying the units crate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove Amount::fmt_value_in

4 participants

@shinghim @coveralls @tcharding @apoelstra