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

Default Cargo.toml template provide help for completing the metadata #6881

Merged
merged 2 commits into from
May 2, 2019

Conversation

k-nasa
Copy link
Contributor

@k-nasa k-nasa commented Apr 26, 2019

Descriptio

This is new Cargo.toml example.

[package]
name = "foo"
version = "0.1.0"
authors = ["k-nasa"]
documentation = ""
homepage = ""
repository = ""
readme = ""
categories = []
keywords = []
license = ""
edition = "2018"

[dependencies]

# See more https://doc.rust-lang.org/cargo/reference/manifest.html

Motivation

close: #6845

@rust-highfive
Copy link

r? @alexcrichton

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 26, 2019
@alexcrichton
Copy link
Member

Thanks for the PR! I would personally prefer to not do this as I feel it clutters up the template with a bit too much noise, but perhaps we could add a comment by default with a link to the reference?

@k-nasa
Copy link
Contributor Author

k-nasa commented Apr 29, 2019

@alexcrichton I fixed it ! How is it like this?
I'm wondering if I write a link on head or write it tail.

@alexcrichton
Copy link
Member

Since the intention was to teach about the metadata in the [package] section, perhaps this could be just after that section and look like:

# See more keys and their definitions at https://...

@k-nasa
Copy link
Contributor Author

k-nasa commented May 1, 2019

I changed it. It will be like this.

[package]
name = "hogehoge"
version = "0.1.0"
authors = ["k-nasa <[email protected]>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

@alexcrichton alexcrichton added the T-cargo Team: Cargo label May 1, 2019
@alexcrichton
Copy link
Member

Ok thanks @k-nasa!

Since this affects the default template which has pretty high visibility I'd like to check in with other folks as well before merging...

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented May 1, 2019

Team member @alexcrichton 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 labels May 1, 2019
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

I think including cargo fmt changes can make it difficult to review PRs. My preference is to only format what the PR touches. Broad-sweeping cargo fmt changes can be done as separate PRs.

@@ -520,5 +520,13 @@ fn new_with_blank_email() {
.run();

let contents = fs::read_to_string(paths::root().join("foo/Cargo.toml")).unwrap();
assert!(contents.contains(r#"authors = ["Sen"]"#), contents);
assert!(contents.contains(r#"authors = ["Sen"]"#));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not know that it was a custom message. I saw document and understood that this change is unnecessary.

@joshtriplett
Copy link
Member

joshtriplett commented May 1, 2019

I like the "just add a comment" version much better. 👍

I'd prefer to see this rebased so it doesn't add-then-remove the fields in successive commits.

@k-nasa k-nasa force-pushed the feature_cargo_toml_template branch from 09469e3 to d3cfdaa Compare May 1, 2019 21:29
@k-nasa k-nasa force-pushed the feature_cargo_toml_template branch from d3cfdaa to f781d2d Compare May 1, 2019 21:30
@k-nasa
Copy link
Contributor Author

k-nasa commented May 1, 2019

Succeeded in rebase.
@ehuss @joshtriplett I'm sorry to do miscellaneous things

@rfcbot rfcbot added final-comment-period FCP — a period for last comments before action is taken and removed proposed-final-comment-period An FCP proposal has started, but not yet signed off. labels May 1, 2019
@rfcbot
Copy link
Collaborator

rfcbot commented May 1, 2019

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

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented May 2, 2019

📌 Commit f781d2d has been approved by alexcrichton

@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 May 2, 2019
@bors
Copy link
Contributor

bors commented May 2, 2019

⌛ Testing commit f781d2d with merge 7eb9e6c...

bors added a commit that referenced this pull request May 2, 2019
Default Cargo.toml template provide help for completing the metadata

## Descriptio
This is new Cargo.toml example.
```toml
[package]
name = "foo"
version = "0.1.0"
authors = ["k-nasa"]
documentation = ""
homepage = ""
repository = ""
readme = ""
categories = []
keywords = []
license = ""
edition = "2018"

[dependencies]

# See more https://doc.rust-lang.org/cargo/reference/manifest.html

```
## Motivation
close: #6845
@bors
Copy link
Contributor

bors commented May 2, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing 7eb9e6c to master...

@bors bors merged commit f781d2d into rust-lang:master May 2, 2019
bors added a commit to rust-lang/rust that referenced this pull request May 7, 2019
Update cargo

12 commits in beb8fcb5248dc2e6aa488af9613216d5ccb31c6a..759b6161a328db1d4863139e90875308ecd25a75
2019-04-30 23:58:00 +0000 to 2019-05-06 20:47:49 +0000
- Small things (rust-lang/cargo#6910)
- Fix skipping over invalid registry packages (rust-lang/cargo#6912)
- Fixes rust-lang/cargo#6874 (rust-lang/cargo#6905)
- doc: Format examples of version to ease reading (rust-lang/cargo#6907)
- fix more typos (codespell) (rust-lang/cargo#6903)
- Parse less JSON on null builds (rust-lang/cargo#6880)
- chore: Update opener to 0.4 (rust-lang/cargo#6902)
- Update documentation for auto-discovery. (rust-lang/cargo#6898)
- Update some doc links. (rust-lang/cargo#6897)
- Default Cargo.toml template provide help for completing the metadata (rust-lang/cargo#6881)
- Run 'cargo fmt --all' (rust-lang/cargo#6896)
- Refactor command definition (rust-lang/cargo#6894)
@rfcbot rfcbot added finished-final-comment-period FCP complete and removed final-comment-period FCP — a period for last comments before action is taken labels May 11, 2019
@ehuss ehuss added this to the 1.36.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In the default Cargo.toml template provide help for completing the metadata
7 participants