-
-
Notifications
You must be signed in to change notification settings - Fork 17.1k
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
doc: add section for project captains #4210
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also add the part about the TC and existing repo captains having a vote?
Contributing.md
Outdated
and the registry, as well as in providing user support. | ||
|
||
To become a captain for a project you will be expected to participate in that | ||
project for at least 6 months prior to the request. You should have helped with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be "for at least 6 months in the role of committer"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we did not specifically call this out in the TC. Should we? because there is an approval process, adding such an additional criteria may be seen as redundant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comment is because I assume that in order to actually function as a repo owner one would need to be a committer? Because if a user had never been a committer before and suddenly was a repo owner, that would seem odd, as one would think that they should at least have non-zero experience landing commits into the repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, the main reasons why it jumped out to me is that the TC is a progression of committer -> TC member, so thought that maybe that would make sense for Repo Captain as well. It just felt natural for some reason, which is the only reason I asked. It also felt like it may help better clarify what "participate" meant if that came up in the future as a point of content (did X actually "participate" for 6 months, etc.). I just felt like if the pre-req felt less fuzzy, a user would be more confident requesting the role by better knowing if they met it or not, idk, haha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So are there no comments on this still? If not, I think the wording should be clarified in this manor. For example, in the TC section, it says that committers elect other committers to the TC. If we do not spell out somewhere that a repo captain is also a committer, then it is not clear that a repo captain can be a TC member unless they also separately become a committer.
I think we should clarify that repo captains are a subset of committers, just like TC members are.
@dougwilson , @wesleytodd : we are ready to land this, isn't it? what is blocking this? |
@dougwilson / @wesleytodd - can you either remove the red-X or clarify what your concern is? If it is just that one is waiting for other's approval, please state so, so that we can move forward. /cc @expressjs/express-tc - FYI, and important PR for defining project captains for repos. Please chime in if you have comments, or else complete review to help progress! |
There is a simple issue here I was going to fix on merge, but didn't get a chance to this weekend with what is going on in my country. After work today I will at least comment on it so anyone can make the change if I cannot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gireeshpunathil these are my notes that I am meaning to fix, but as per your request on what the hold up is, I have written them down here in case you wanted to fix them. I am willing to do so, though. LMK what you want to do.
Contributing.md
Outdated
and the registry, as well as in providing user support. | ||
|
||
To become a captain for a project you will be expected to participate in that | ||
project for at least 6 months prior to the request. You should have helped with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So are there no comments on this still? If not, I think the wording should be clarified in this manor. For example, in the TC section, it says that committers elect other committers to the TC. If we do not spell out somewhere that a repo captain is also a committer, then it is not clear that a repo captain can be a TC member unless they also separately become a committer.
I think we should clarify that repo captains are a subset of committers, just like TC members are.
@dougwilson - I have addressed all the review comments, mostly following your suggestions |
it has been 11 days since the last update. can we take this forward? |
6d50cd5
to
f628080
Compare
@dougwilson - I just squashed all the commits into one |
Thanks @gireeshpunathil ! As we discussed on the TC meeting, it should be in the format as the current commit on |
f628080
to
1d73579
Compare
@dougwilson - made the changes to the commit message as per the guidelines; PTAL. |
Ok... It still does not seem to be in the form from my comment... ? I can just update if you need, let me know. If I can better clarify how the message should be constructed let me know as well. I believe it was clear, and provided an example of a previous comment message, but if I can provide better guidance for the format, I certainly can. I am circling around on this because I would like this to get merged, but I recall that you wanted to do the commit editing so I could just do the fast-forward merge, but it's not in the correct state for that. I can make those edits if you like, but don't want to force push on your branch without you being aware as per our previous conversation :) |
this is the current commit message:
this is the rule:
the two additional things are:
please advise where is the change required? |
It should just be something like
Then as discussed in the TC meeting, the committer on the commit would need to be the person performing the merge itself (myself unless you would like to wait for another to do that). |
1d73579
to
8976172
Compare
@dougwilson - adjusted the commit message accordingly, PTAL! |
So based on the conversation going on in expressjs/discussions#121 it seems like this is not ready to land as-is, as it seems that there may be either additional discussion to have on the goal here and/or updating to the wording in this pull request to reflect it. Specifically in regards to what falls under repo ownership. It would come to reason to me that landing pull requests would be a "day-to-day" task for said project captains, and their particular merging strategies, commit message formats, etc. would serve the repo on a technical front, as outlined in this document. I think part of the goal of project captains is to empower them to manage their repos, and commit messages seems to fall directly in that category to me. For example, if a project captain wants to use something like standard-release to automate releases and history file generation, they may need specific structure to commit messages. We should strive to empower the owners to do what they think will improve their workflow. But maybe I'm misunderstanding on that point. Especially if I'm misunderstanding, that definately means we need greater clarity in the document, as we don't want to have everyone interpreting it differently, I presume. I'm happy to provide suggested edits on the wording, but likely need input from the original author @wesleytodd to sync on the interpretation so I can help clarify 👍 |
@dougwilson - it seems strange (and funny) to me that you closed expressjs/discussions#121 with no action on it, and then quoting its content as a reason for further work here! |
Hi @gireeshpunathil that issue is orthogonal to this issue. |
here is the sequence of events, for better clarity for everyone:
This is becoming a surprise to me! where is the mis-understanding? If you assert that no org wide guidelines are present, so be it, and I will seek change proposal with captains, period! where is it that I interpreted this document at all? leave alone interpreted differently? I suggest we take a step back;
|
Hi @gireeshpunathil please let me know if there is some time in which you are available for a voice chat; I would like to proceed with landing this PR, but I am honestly unsure at this point how to interact with your pull requests that will not come off in bad faith, and would like a chance to have a discussion with you so we are on the same page so we can make progress landing. |
@dougwilson - I sent you a mail with my time preferences. |
in general, this can have more reviews! |
I don't have any real comments on this PR, but once the discussion is resolved and it lands, we should make sure the change is reflected on the website; I opened expressjs/expressjs.com#1184 to track that. |
I believe I have addressed all the outstanding comments. pinging @expressjs/express-tc , to seek some reviews or take it towards landing. |
I am locking this PR until the above issue with the interaction between myself and @gireeshpunathil is resolved. |
eb10dba
to
340be0f
Compare
Closing in favor of #5484. |
Refs: expressjs/discussions#98
Refs: expressjs/discussions#106 (comment)
co-authored-by: Wes Todd @wesleytodd