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

doc: update author-ready label terms #23249

Closed
wants to merge 1 commit into from
Closed

doc: update author-ready label terms #23249

wants to merge 1 commit into from

Conversation

vsemozhetbyt
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

If I understand it correctly, we need this correction after #22255

cc @Trott

@vsemozhetbyt vsemozhetbyt added the meta Issues and PRs related to the general management of the project. label Oct 3, 2018
@nodejs-github-bot
Copy link
Collaborator

@vsemozhetbyt build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1117/pipeline

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 3, 2018
@refack
Copy link
Contributor

refack commented Oct 3, 2018

Is there wording about authors who wish to land their own PRs?
i.e. Only the author should set author-ready?

@vsemozhetbyt
Copy link
Contributor Author

Sorry to ping @nodejs/collaborators, but it seems this is worth PSA.

Does this need to be fixed or left as is?

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 5, 2018
@vsemozhetbyt
Copy link
Contributor Author

Is there wording about authors who wish to land their own PRs?
i.e. Only the author should set author-ready?

@refack Can you elaborate? I do not recall any rules re who can or should add the label. Do you suggest a restriction or just try to clear up the status quo?

@refack
Copy link
Contributor

refack commented Oct 5, 2018

Do you suggest a restriction or just try to clear up the status quo?

In the context of this PR I'm mostly asking; is there an agreed way to flag PRs that the author wished to land themselves?
If there isn't IMHO it's worth discussing (in a new PR).

@vsemozhetbyt
Copy link
Contributor Author

Sorry, I do not know about any such flagging(

@gireeshpunathil
Copy link
Member

Given that this is collaborator's guide and the situation of author landing own PRs arises only to collaborators, that discussion (@refack 's) is most relevant here itself IMO. However, I don't see the rules, pre-conditions, authority for setting or unsetting the author-ready label need not be any different for the author collaborator from any other collaborator.

@refack refack mentioned this pull request Oct 6, 2018
Merged
3 tasks
@vsemozhetbyt
Copy link
Contributor Author

Landed in 1a18e35
Thank you for the reviews.

@vsemozhetbyt vsemozhetbyt deleted the author-ready-2-approvals branch October 7, 2018 09:00
vsemozhetbyt added a commit that referenced this pull request Oct 7, 2018
@vsemozhetbyt
doc: update author-ready label terms
1a18e35 
PR-URL: #23249
Refs: #22255
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
targos pushed a commit that referenced this pull request Oct 7, 2018
@vsemozhetbyt @targos
doc: update author-ready label terms
d726c31 
PR-URL: #23249
Refs: #22255
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
targos pushed a commit that referenced this pull request Oct 7, 2018
@vsemozhetbyt @targos
doc: update author-ready label terms
11c6745 
PR-URL: #23249
Refs: #22255
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
@targos targos mentioned this pull request Oct 7, 2018
Merged
This was referenced Oct 10, 2018
Closed
Closed
Closed
Closed
Closed
This was referenced Oct 11, 2018
Closed
Closed
Closed
Closed
Closed
refack added a commit to refack/node that referenced this pull request Oct 13, 2018
@refack
doc,meta: refresh wording in colab guide
39ae000 
PR-URL: nodejs#23292
Refs: nodejs#23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
refack added a commit to refack/node that referenced this pull request Oct 13, 2018
@refack
doc,meta: assign PR semantics
fc0da7f 
PR-URL: nodejs#23292
Refs: nodejs#23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
addaleax pushed a commit that referenced this pull request Oct 14, 2018
@refack @addaleax
doc,meta: refresh wording in colab guide
34ebbfd 
PR-URL: #23292
Refs: #23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
addaleax pushed a commit that referenced this pull request Oct 14, 2018
@refack @addaleax
doc,meta: assign PR semantics
a7ed986 
PR-URL: #23292
Refs: #23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
jasnell pushed a commit that referenced this pull request Oct 17, 2018
@vsemozhetbyt @jasnell
doc: update author-ready label terms
8875863 
PR-URL: #23249
Refs: #22255
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
jasnell pushed a commit that referenced this pull request Oct 17, 2018
@refack @jasnell
doc,meta: refresh wording in colab guide
d08544f 
PR-URL: #23292
Refs: #23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
jasnell pushed a commit that referenced this pull request Oct 17, 2018
@refack @jasnell
doc,meta: assign PR semantics
72258c3 
PR-URL: #23292
Refs: #23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 30, 2018
@refack @MylesBorins
doc,meta: refresh wording in colab guide
b7d6c1a 
PR-URL: #23292
Refs: #23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 30, 2018
@refack @MylesBorins
doc,meta: assign PR semantics
26f4221 
PR-URL: #23292
Refs: #23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
@refack @rvagg
doc,meta: refresh wording in colab guide
68b6f86 
PR-URL: #23292
Refs: #23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
@refack @rvagg
doc,meta: assign PR semantics
d1fcdc2 
PR-URL: #23292
Refs: #23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
@refack @MylesBorins
doc,meta: refresh wording in colab guide
2ddf867 
PR-URL: #23292
Refs: #23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
@refack @MylesBorins
doc,meta: assign PR semantics
73f490a 
PR-URL: #23292
Refs: #23249
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@BridgeAR
Copy link
Member

BridgeAR commented Dec 2, 2018

This was briefly discussed by @addaleax and me about this and I don't think that #22255 implies this change. See https://github.com/orgs/nodejs/teams/collaborators/discussions/49?from_comment=1#discussion-49-comment-1. I would like to revert this since it makes it pretty hard to apply the label early and that is the main benefit, I personally got from it and I guess that could likely also apply to others.

This label was also applied for semver-major PRs when the @nodejs/tsc LGs where still outstanding. This is another reason why this would not imply this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trivikr trivikr trivikr approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@Trott Trott Trott approved these changes

@thefourtheye thefourtheye thefourtheye approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@vsemozhetbyt @nodejs-github-bot @refack @gireeshpunathil @BridgeAR @thefourtheye @Trott @trivikr