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

Contribution guide does not mention our "no merge commits" policy #59233

Closed
oli-obk opened this issue Mar 16, 2019 · 5 comments · Fixed by #65007
Closed

Contribution guide does not mention our "no merge commits" policy #59233

oli-obk opened this issue Mar 16, 2019 · 5 comments · Fixed by #65007
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. metabug Issues about issues themselves ("bugs about bugs")

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Mar 16, 2019

We expect everyone to rebase instead of merge and we also try to have fixup commits squashed into previous related commits. I think we should document this policy, since it keeps suprising newcomers.

@stepnivlk
Copy link
Contributor

@oli-obk I can take care of this one as it's quite a fresh experience :).

@tesuji
Copy link
Contributor

tesuji commented Mar 16, 2019

I think we should only rebase when resolving merge conflict, or the review process has completed
and reviewer request a rebasing.
Otherwise I prefer adding new commits for easier reviewing.

@bovinebuddha
Copy link
Contributor

@oli-obk

I agree with @lzutao that this policy is sound for actually merging the PR, but it might cause confusion for reviews. I've had a PR lying around for awhile, and when there is merge conflicts the 'waiting-on-author' label gets added so noone reviews it, but when I do the rebasing to get back my 'waiting-on-review' it seems as if Github sometimes loses context of previous reviews, which can be quite confusing...

@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 17, 2019

for the record: I personally want no rebasing and just merging, I'm just stating what the policy is.

Centril added a commit to Centril/rust that referenced this issue Mar 31, 2019
…i-obk

Mention `no merge policy` in the CONTRIBUTING guide

Issue: rust-lang#59233
Centril added a commit to Centril/rust that referenced this issue Mar 31, 2019
…i-obk

Mention `no merge policy` in the CONTRIBUTING guide

Issue: rust-lang#59233
Centril added a commit to Centril/rust that referenced this issue Mar 31, 2019
…i-obk

Mention `no merge policy` in the CONTRIBUTING guide

Issue: rust-lang#59233
@tesuji
Copy link
Contributor

tesuji commented Mar 31, 2019

Before we close this issue. Could we talk a bit about closing issues using keyword?

For example, to close an issue numbered 123, you could use the phrase "Closes #123" or "Closes: #123" in your pull request description or commit message. Once the branch is merged into the default branch, the issue will close.

I think we should encourage contributors close issues by keywords in PR descriptions only.
Because otherwise the commit could be rebased many times, and each time it creates a reference to
the want-to-close issue. 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.

In short, we create another policy for closing issue.

@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. metabug Issues about issues themselves ("bugs about bugs") A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Apr 21, 2019
@bors bors closed this as completed in bd82de0 Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. metabug Issues about issues themselves ("bugs about bugs")
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants