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

Mention keyword closing policy #65007

Merged
merged 3 commits into from Oct 23, 2019
Merged

Mention keyword closing policy #65007

merged 3 commits into from Oct 23, 2019

Conversation

ghost
Copy link

@ghost ghost commented Oct 2, 2019

closes #59233 / #59233 (comment)

rewording suggestions welcome

Also in the referenced issue, the commit number of the new commit
that could close that issue is not really informative. The PR number itself appeared in the issue
is more informative and concise.

@lzutao what do you mean with that? Is this fixed by the new "May be fixed by #XXXXX"?

@rust-highfive
Copy link
Collaborator

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 Oct 2, 2019
@tesuji
Copy link
Contributor

tesuji commented Oct 2, 2019

Yes, i think so.

Centril
Centril reviewed Oct 2, 2019
View reviewed changes
CONTRIBUTING.md Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Oct 2, 2019

r? @oli-obk

--- I think we may want compiler team FCP on this?

I've personally found it useful to ping certain old issues in my commits so that I remember them. For example, in https://github.com/rust-lang/rust/pull/64221/commits I referenced a bunch of issues that I wanted to close.

@rust-highfive rust-highfive assigned oli-obk and unassigned alexcrichton Oct 2, 2019
@oli-obk oli-obk added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 2, 2019
@JohnTitor JohnTitor added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Oct 13, 2019
nikomatsakis
nikomatsakis reviewed Oct 17, 2019
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

I don't have a strong opinion here, but I think if we're going to word this, we might as well explain our reasoning.

CONTRIBUTING.md Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Oct 17, 2019

My view is that we shouldn't legislate here. It is sometimes useful to reference issues in commit messages purely to remember them whereas I would have to have post-it notes to remember them for when I make the PR. Also, it's worth noting that most of the "spam" comes from @bors's commit messages and those are useful when looking over blame.

@ghost
Copy link
Author

ghost commented Oct 17, 2019

I am fine with both.
Please decide an close this PR and #59233 if this policy is not wanted 😉

@Centril
Copy link
Contributor

Centril commented Oct 18, 2019

r? @nikomatsakis

Apply suggested wording
1c85b45 
to better describe the situation
@nikomatsakis
Copy link
Contributor

@bors r+ rollup

seems good now

@bors
Copy link
Contributor

bors commented Oct 22, 2019

📌 Commit 1c85b45 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Oct 22, 2019

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@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. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Oct 22, 2019
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 23, 2019
@JohnTitor
Rollup merge of rust-lang#65007 - BO41:keywords, r=nikomatsakis
bd82de0 
Mention keyword closing policy

closes rust-lang#59233 / rust-lang#59233 (comment)

rewording suggestions welcome

> Also in the referenced issue, the commit number of the new commit
> that could close that issue is not really informative. The PR number itself appeared in the issue
> is more informative and concise.

@lzutao what do you mean with that? Is this fixed by the new "May be fixed by #XXXXX"?
@JohnTitor JohnTitor mentioned this pull request Oct 23, 2019
Merged
bors added a commit that referenced this pull request Oct 23, 2019
@bors
Auto merge of #65716 - JohnTitor:rollup-fkcr85k, r=JohnTitor
f466f52 
Rollup of 14 pull requests

Successful merges:

 - #64145 (Target-feature documented as unsafe)
 - #65007 (Mention keyword closing policy)
 - #65417 (Add more coherence tests)
 - #65507 (Fix test style in unused parentheses lint test)
 - #65591 (Add long error explanation for E0588)
 - #65617 (Fix WASI sleep impl)
 - #65656 (Add option to disable keyboard shortcuts in docs)
 - #65678 (Add long error explanation for E0728)
 - #65681 (Code cleanups following up on #65576.)
 - #65686 (refactor and move `maybe_append` )
 - #65688 (Add some tests for fixed ICEs)
 - #65689 (bring back some Debug instances for Miri)
 - #65695 (self-profiling: Remove module names from some event-ids in codegen backend.)
 - #65706 (Add missing space in librustdoc)

Failed merges:

r? @ghost
@bors bors merged commit 1c85b45 into rust-lang:master Oct 23, 2019
@ghost ghost deleted the keywords branch October 23, 2019 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Contribution guide does not mention our "no merge commits" policy
8 participants
@rust-highfive @tesuji @Centril @nikomatsakis @bors @alexcrichton @oli-obk @JohnTitor