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

refactor: use RegistryOrIndex enum to replace two booleans #12677

Merged
merged 4 commits into from
Sep 17, 2023

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

fn registry() has two booleans representing --index and --registry respectively. However, they are inherently mutually exclusive and better to use a single sum type RegistryOrIndex to model them.

How should we test and review this PR?

This PR is expected not to bring any behavior change. The only change is that the cli arg conflict detection is now moved to command line parsing phase (clap).

A new test publish_failed_with_index_and_only_allowed_registry demonstrates a quirky message was emitted when

  • package.publish = […] contains the only one registry that trigger an implicit publish
  • --index arg was provided and took precedence over the implicit registry publish

This behavior should be fixed IMO but let's leave it to another PR.

Additional information

@rustbot
Copy link
Collaborator

rustbot commented Sep 15, 2023

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. A-interacts-with-crates.io Area: interaction with registries Command-init Command-install Command-login Command-logout Command-new Command-owner Command-publish Command-search Command-yank S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 15, 2023
Copy link
Member

@Rustin170506 Rustin170506 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. Thanks!

@ehuss
Copy link
Contributor

ehuss commented Sep 17, 2023

Thanks!

@bors r=hi-rustin

@bors
Copy link
Contributor

bors commented Sep 17, 2023

📌 Commit 11754e9 has been approved by hi-rustin

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 Sep 17, 2023
@bors
Copy link
Contributor

bors commented Sep 17, 2023

⌛ Testing commit 11754e9 with merge 41cef47...

@bors
Copy link
Contributor

bors commented Sep 17, 2023

☀️ Test successful - checks-actions
Approved by: hi-rustin
Pushing 41cef47 to master...

@bors bors merged commit 41cef47 into rust-lang:master Sep 17, 2023
19 checks passed
@weihanglo weihanglo deleted the registry-or-index branch September 17, 2023 23:44
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 18, 2023
Update cargo

6 commits in d5336f813df39d476e61fc46daabb1446350660a..b4ddf95ad9954118ac0dae835f2966394ad04c02
2023-09-14 19:55:49 +0000 to 2023-09-18 03:48:09 +0000
- doc: differentiate defaults for split-debuginfo (rust-lang/cargo#12680)
- feat(cli): Add '-n' to dry-run (rust-lang/cargo#12660)
- feat: stabilize credential-process and registry-auth (rust-lang/cargo#12649)
- refactor: use `RegistryOrIndex` enum to replace two booleans (rust-lang/cargo#12677)
- doc: clarify caret requirements (rust-lang/cargo#12679)
- feat(pkgid): Allow incomplete versions when unambigious (rust-lang/cargo#12614)

r? ghost
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. A-interacts-with-crates.io Area: interaction with registries Command-init Command-install Command-login Command-logout Command-new Command-owner Command-publish Command-search Command-yank S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants