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.

Sign up for GitHub

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

Jump to bottom

Improve crate manifests, adding missing [package.repository] and [package.description] fields #17745

Merged
merged 9 commits into from
Aug 6, 2024
Merged

Improve crate manifests, adding missing [package.repository] and [package.description] fields #17745

merged 9 commits into from
Aug 6, 2024

Conversation

regexident
Copy link
Contributor

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 30, 2024
lnicola
lnicola reviewed Jul 30, 2024
View reviewed changes
crates/base-db/Cargo.toml Outdated
@@ -1,7 +1,8 @@
[package]
name = "base-db"
version = "0.0.0"
description = "TBD"
repository = "https://github.com/rust-lang/rust-analyzer"
description = "Basic database traits. The concrete DB is defined by `ra_ap_ide`."
Copy link
Member

Choose a reason for hiding this comment

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

Bah, feels like this should say ide.

Copy link
Contributor Author

@regexident regexident Jul 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I changed it to ra_ap_ide as that's the name that the crate is currently published as. As such anyone reading the description on crates.io would be tempted to search for ide but not find anything (or worse even, be lead to a completely different unrelated and mostly empty ide crate).

Happy to change it back to ide though, if that's the consensus of what it should be referred to as.

Copy link
Member

Choose a reason for hiding this comment

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

Can we just do something like ide/ra_ap_ide then?

lnicola
lnicola reviewed Jul 30, 2024
View reviewed changes
crates/hir/Cargo.toml Outdated Show resolved Hide resolved
lnicola
lnicola reviewed Jul 30, 2024
View reviewed changes
crates/ide-db/Cargo.toml Outdated Show resolved Hide resolved
@lnicola
Copy link
Member

lnicola commented Jul 30, 2024

It's a bit confusing how half of these say "for rust-analyzer" and half don't.

@regexident
Copy link
Contributor Author

regexident commented Jul 30, 2024 •
edited
Loading

It's a bit confusing how half of these say "for rust-analyzer" and half don't.

Agreed. I was leaning towards adding it to give people stumbling upon a ra_ap_ crate a little bit of context.
Otherwise "The type system." or the like (as taken verbatim from the crate's lib.rs docs) isn't much to hold on to.

For others adding it felt a bit verbose though, so I omitted it as I wanted to stay true to the crate's module docs.

Having an inconsistent mix definitely isn't optimal though as it begs the question of "is crate ra_ap_… not for rust analyzer?".

I'm happy with whatever you —the maintainers— prefer.

@regexident
Copy link
Contributor Author

I'm happy with whatever you —the maintainers— prefer.

@Veykril do you have an opinion on this (as well as the ide vs ra_ap_ide topic above)?

Veykril
Veykril reviewed Aug 1, 2024
View reviewed changes
crates/hir-def/Cargo.toml Outdated
@@ -1,7 +1,8 @@
[package]
name = "hir-def"
version = "0.0.0"
description = "TBD"
repository = "https://github.com/rust-lang/rust-analyzer"
description = "Everything between macro expansion and type inference for rust-analyzer."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description = "Everything between macro expansion and type inference for rust-analyzer."
description = "Early name and import resolution for rust-analyzer."

might be more fitting

Veykril
Veykril reviewed Aug 1, 2024
View reviewed changes
crates/proc-macro-api/Cargo.toml Outdated
@@ -1,7 +1,8 @@
[package]
name = "proc-macro-api"
version = "0.0.0"
description = "TBD"
repository = "https://github.com/rust-lang/rust-analyzer"
description = "Client-side proc-macros for rust-analyzer."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description = "Client-side proc-macros for rust-analyzer."
description = "RPC Api for the `proc-macro-srv`.

Veykril
Veykril reviewed Aug 1, 2024
View reviewed changes
crates/syntax/Cargo.toml Outdated
repository = "https://github.com/rust-lang/rust-analyzer"
description = "Comment and whitespace preserving parser for the Rust language"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description = "Comment and whitespace preserving parser for the Rust language"
description = "Concrete syntax tree definitions for rust-analyzer"

@Veykril
Copy link
Member

Veykril commented Aug 2, 2024

@bors delegate+

@bors
Copy link
Contributor

bors commented Aug 2, 2024

✌️ @regexident, you can now approve this pull request!

If @Veykril told you to "r=me" after making some further change, please make that change, then do @bors r=@Veykril

@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 2, 2024
@Veykril
Copy link
Member

Veykril commented Aug 2, 2024

not sure whats up with CI, it runs on nightly given the changes so there might be something breaking for us on latest nightly

@regexident regexident force-pushed the improve-crate-manifests branch from 5f25256 to 2bd7ed1 Compare August 3, 2024 15:52
@regexident
Copy link
Contributor Author

@Veykril FYI I've applied your suggested changes and also added any missing mentions of rust-analyzer (cc @lnicola ) with the following reasoning:

With the lack of a README on the individually published library crates and the somewhat cryptic ra_ap_ prefix it is hard to figure out where those crates belong to, so mentioning "rust-analyzer" feels like a useful hint there. I've tried to make the name-drop fit into the description without sticking out too much, so it's not always a suffix.

This is quite a bit more than your suggested changes though, so …

@bors r=@Veykril

… just to be sure you're good with those changes. The PR can be merged you're happy with it.

@bors
Copy link
Contributor

bors commented Aug 3, 2024

📌 Commit 2bd7ed1 has been approved by Veykril

It is now in the queue for this repository.

bors added a commit that referenced this pull request Aug 3, 2024
@bors
Auto merge of #17745 - regexident:improve-crate-manifests, r=Veykril
59d8786 
Improve crate manifests, adding missing `[package.repository]` and `[package.description]` fields

As [discussed on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Frust-analyzer/topic/Could.20we.20add.20repository.20url.20to.20.60ra_ap_.60.20crates.20on.20crates.2Eio.3F/near/455095161).

cc `@Veykril` `@lnicola`
@bors
Copy link
Contributor

bors commented Aug 3, 2024

⌛ Testing commit 2bd7ed1 with merge 59d8786...

@bors
Copy link
Contributor

bors commented Aug 3, 2024

💔 Test failed - checks-actions

Veykril
Veykril reviewed Aug 5, 2024
View reviewed changes
crates/base-db/Cargo.toml Outdated
@@ -1,7 +1,8 @@
[package]
name = "base-db"
version = "0.0.0"
description = "TBD"
repository = "https://github.com/rust-lang/rust-analyzer"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
repository = "https://github.com/rust-lang/rust-analyzer"
repository.workspace = true

Nit, this should be workspace inehritable no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that you mention it, yes of course! Fixed.

@Veykril
Copy link
Member

Veykril commented Aug 5, 2024

CI should succeed again after #17794 is merged

@regexident
Unify package descriptions by adding references to "rust-analyzer"
624f2ea 
With the lack of a README on the individually published library crates and the somewhat cryptic `ra_ap_` prefix it is hard to figure out where those crates belong to, so mentioning "rust-analyzer" feels like auseful hint there.
@regexident regexident force-pushed the improve-crate-manifests branch from 72dd3aa to 868deab Compare August 5, 2024 22:25
@regexident regexident force-pushed the improve-crate-manifests branch from 868deab to 7dec7e9 Compare August 5, 2024 22:27
@Veykril
Copy link
Member

Veykril commented Aug 6, 2024

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Aug 6, 2024

📌 Commit 7dec7e9 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 6, 2024

⌛ Testing commit 7dec7e9 with merge b231422...

@bors
Copy link
Contributor

bors commented Aug 6, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing b231422 to master...

@bors bors merged commit b231422 into rust-lang:master Aug 6, 2024
11 checks passed
@regexident regexident deleted the improve-crate-manifests branch August 6, 2024 07:16
@lnicola
Copy link
Member

lnicola commented Aug 6, 2024

@regexident there's one more if you want (syntax-bridge).

@regexident regexident mentioned this pull request Aug 7, 2024
Merged
bors added a commit that referenced this pull request Aug 8, 2024
@bors
Auto merge of #17827 - regexident:improve-crate-manifests-continuatio…
b02c6bf 
…n, r=lnicola

Improve crate manifest of 'syntax-bridge', adding missing `[package.repository]` and `[package.description]` fields

This is a follow-up of #17745, specifically [this comment](#17745 (comment)) by `@lnicola.`

It refines the manifest of the newly added 'syntax-bridge' crate, adding a `[package.repository]` as `workspace = true` and changes the existing `[package.description]` from "TBD" to a more useful description.
lnicola pushed a commit to lnicola/rust that referenced this pull request Aug 13, 2024
@bors
Auto merge of rust-lang#17827 - regexident:improve-crate-manifests-co…
fe4d83c 
…ntinuation, r=lnicola

Improve crate manifest of 'syntax-bridge', adding missing `[package.repository]` and `[package.description]` fields

This is a follow-up of rust-lang/rust-analyzer#17745, specifically [this comment](rust-lang/rust-analyzer#17745 (comment)) by `@lnicola.`

It refines the manifest of the newly added 'syntax-bridge' crate, adding a `[package.repository]` as `workspace = true` and changes the existing `[package.description]` from "TBD" to a more useful description.
@regexident regexident mentioned this pull request Aug 14, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lnicola lnicola lnicola left review comments

@Veykril Veykril Veykril approved these changes

Assignees
No one assigned
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@regexident @lnicola @Veykril @bors @rustbot