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(toml)!: Remove support for inheriting badges #13788

Merged
merged 1 commit into from
Apr 29, 2024
Merged

Conversation

epage
Copy link
Contributor

@epage epage commented Apr 22, 2024

What does this PR try to resolve?

We allowed [badges] to inherit from [workspace.package.badges] which was a bug:

  • This was not specified in the RFC
  • We did not document this
  • Even if someone were to try to guess to use this, it is inconsistent with how inheritance works because this should inherit from workspace.badges instead of workspace.package.badges

While keeping in mind that [badges] is effectively deprecated.

In that context, I think its safe to break support for this without a transition period.

Fixes #13643

How should we test and review this PR?

Additional information

We allowed `[badges]` to inherit from `[workspace.package.badges]`

This was a bug:
- This was not specified in the RFC
- We did not document this
- Even if someone were to try to guess to use this, it is inconsistent
  with how inheritance works because this should inherit from
  `workspace.badges` instead of `workspace.package.badges`

While keeping in mind that `[badges]` is effectively deprecated.

In that context, I think its safe to break support for this without a
transition period.

Fixes rust-lang#13643
@rustbot
Copy link
Collaborator

rustbot commented Apr 22, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-manifest Area: Cargo.toml issues S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 22, 2024
@weihanglo weihanglo added the T-cargo Team: Cargo label Apr 23, 2024
@weihanglo
Copy link
Member

As this is sort of a breaking change to an undocumented behavior that shouldn't have happened for a deprecated field, I'd like to start a poll.

(not going to wait for a full 10-days FCP after we reach a consensus)

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 23, 2024

Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge final-comment-period FCP — a period for last comments before action is taken labels Apr 23, 2024
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 26, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period An FCP proposal has started, but not yet signed off. label Apr 26, 2024
@epage epage force-pushed the badges branch 4 times, most recently from 2b87813 to bdd4bda Compare April 29, 2024 15:45
@weihanglo
Copy link
Member

@bors r+

Thanks for the update.

@bors
Copy link
Contributor

bors commented Apr 29, 2024

📌 Commit bdd4bda has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 29, 2024
@bors
Copy link
Contributor

bors commented Apr 29, 2024

⌛ Testing commit bdd4bda with merge cc20b55...

@bors
Copy link
Contributor

bors commented Apr 29, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing cc20b55 to master...

@bors bors merged commit cc20b55 into rust-lang:master Apr 29, 2024
23 checks passed
@epage epage deleted the badges branch April 29, 2024 17:24
bors added a commit to rust-lang-ci/rust that referenced this pull request May 1, 2024
Update cargo

15 commits in b60a1555155111e962018007a6d0ef85207db463..6087566b3fa73bfda29702632493e938b12d19e5
2024-04-26 16:37:29 +0000 to 2024-04-30 20:45:20 +0000
- fix(cargo-fix): dont fix into standard library (rust-lang/cargo#13792)
- refactor: Move diagnostic printing to Shell (rust-lang/cargo#13813)
- Populate git information when building Cargo from Rust's source tarball (rust-lang/cargo#13832)
- docs: fix several typos found by `typos-cli` (rust-lang/cargo#13831)
- fix(alias): Aliases without subcommands should not panic (rust-lang/cargo#13819)
- fix(toml): Improve granularity of traces (rust-lang/cargo#13830)
- fix(toml): Warn, rather than fail publish, if a target is excluded (rust-lang/cargo#13713)
- test(cargo-lints): Add a test to ensure cap-lints works (rust-lang/cargo#13829)
- fix(toml)!: Remove support for inheriting badges (rust-lang/cargo#13788)
- chore(ci): Don't check `cargo` against beta channel (rust-lang/cargo#13827)
- Fix target entry in .gitignore (rust-lang/cargo#13817)
- Bump to 0.81.0; update changelog (rust-lang/cargo#13823)
- Add failing test: artifact_dep_target_specified (rust-lang/cargo#13816)
- fix(cargo-lints): Don't always inherit workspace lints (rust-lang/cargo#13812)
- Update SleepTraker returns_in_order unit test (rust-lang/cargo#13811)

r? ghost
@rustbot rustbot added this to the 1.80.0 milestone May 1, 2024
@rfcbot rfcbot added to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-manifest Area: Cargo.toml issues disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Undocumented, inconsisten inheritance of [badges]
5 participants