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

Contradicting merging policy rules #5148

Closed
wouterj opened this issue Apr 5, 2015 · 0 comments
Closed

Contradicting merging policy rules #5148

wouterj opened this issue Apr 5, 2015 · 0 comments
Labels
actionable Clear and specific issues ready for anyone to take them. good first issue Ideal for your first contribution! (some Symfony experience may be required) hasPR A Pull Request has already been submitted for this issue.

Comments

@wouterj
Copy link
Member

wouterj commented Apr 5, 2015

http://symfony.com/doc/current/contributing/code/core_team.html#pull-request-merging-policy

A pull request can be merged if:

  • Enough time was given for peer reviews (a few minutes for typos or minor changes, at least 2 days for "regular" pull requests, and 4 days for pull requests with "a significant impact");
  • It is a minor change [1], regardless of the number of votes;

Rule (1) says that minor changes need a few minutes of peer review time, while rule (2) states that it can be merged immediately.

Besides this, "a few minutes for typos or minor changes" is redundant, as typos are already included in minor changes according to footnote 1 ("Minor changes comprise typos, DocBlock fixes, code standards violations, and minor CSS, JavaScript and HTML modifications.").

At last, the comma before "and" in footnote 1 has to be removed.
@wouterj wouterj added good first issue Ideal for your first contribution! (some Symfony experience may be required) actionable Clear and specific issues ready for anyone to take them. labels Apr 5, 2015
@lscholten lscholten mentioned this issue Apr 30, 2015
Merged
@wouterj wouterj added the hasPR A Pull Request has already been submitted for this issue. label May 10, 2015
wouterj added a commit that referenced this issue May 10, 2015
@wouterj
minor #5219 Fix contradicting merging policy rules (lscholten)
a9445c9 
This PR was merged into the 2.3 branch.

Discussion
----------

Fix contradicting merging policy rules

| Q                     | A
| ------------------ | ---
| Doc fix?           | yes
| New docs?      | no
| Applies to        | 2.3+
| Fixed tickets    | #5148

I've removed "a few minutes for typos or minor changes" from the first rule, fixing the contradiction and the redundancy.
I've also reordered the rules to remove "regardless of the number of votes" from the minor change rule.

Commits
-------

da57e09 Fix contradicting merge policy rules
@wouterj wouterj closed this as completed May 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
actionable Clear and specific issues ready for anyone to take them. good first issue Ideal for your first contribution! (some Symfony experience may be required) hasPR A Pull Request has already been submitted for this issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@wouterj