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

Co-authored-by field in commit message #121

Closed
gireeshpunathil opened this issue Apr 15, 2020 · 17 comments
Closed

Co-authored-by field in commit message #121

gireeshpunathil opened this issue Apr 15, 2020 · 17 comments
Labels

Comments

@gireeshpunathil
Copy link

In a highly collaborative development process, it is quite natural to have multiple authors for a piece of contribution artifact. The current commit guidelines do not have a clause for this.

github has a workflow that caters to this use case: https://help.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors

Proposal: introduce co-authored-by field in the commit message, to be used wherever appropriate.

goal: attribute authorship appropriately, and fairly.

Refs: expressjs/express#4210 (comment) , point 3.

@dougwilson
Copy link
Contributor

I would think that, especially in light of project captains, that the TC likely should not be specifically the exact format of commits for every repo under it's reach. Repos do not currently have a unified approach to commit messages, merges, etc. So I would think that if this is something that should be under TC jurisdiction, then we should determine that first before discussing specific commit policies. I would say that it likely should not fall under the TC and just leave it up to the repos themselves, especially in light of #119 :)

@gireeshpunathil
Copy link
Author

@dougwilson - I missed to follow a few:

that the TC likely should not be specifically the exact format of commits for every repo under it's reach.

do you mean TC likely should not be specifically concerned about ?

For the rest: bringing this topic under TC's jurisdiction is not my intention, but defining a uniform guideline is always good IMO - or else we run the risk of 90+ repos defining individual rules leading to utter confusion for folks who cross-collaborate.

@dougwilson
Copy link
Contributor

but defining a uniform guideline is always good IMO - or else we run the risk of 90+ repos defining individual rules leading to utter confusion for folks who cross-collaborate.

Definately I can see that--there has been attempts in the past, but they have not been successful, haha. But even without that, I think with the project captains proposal, it would seem we are going in the direction to allow day-to-day decisions to be that of the given owners. It calls out that they are empowered to own the repositories, and I would think that how the technical details of the repo is run seems very much under the captain's jurisdiction to determine...

I think if we do want to go this route and see it successful, we likely need to carve out from the project captains proposal that they may be expected to follow some technical details provided from a central governance, but anything not specified above they are free to own.

If you disagree on that, I think we should probably go back to the project captains proposal and better clarify on what "repo ownership" actually means. The format of commit messages, merge strategies, etc. seem like technical details of a repo that seem to be pushed down into the hands of project captains and away from a certain guidelines.

I can see pros/cons with both approaches, and don't have a strong one on which we take. My opinion would just be that we should work to set up our guidelines to work with the method we want to do, and so I'm just raising an issue here in that this seems to conflict with the direction of project captains as it's currently written, imo

@gireeshpunathil
Copy link
Author

IMO project captain aims to address a different goal - to decentralize the work, reduce the dependency, and take decisions for day-to-day work.

guidelines are best practice recommendations that over-arch the entire project, not rules. They don't limit project captain's ability to work in any manner, rather provide better guidance. Take the example of current case: you wanted me to remove the tag, quoting that it is not present in the guidelines. Having an agreed set of guidelines that are modern, fair and open, will help project captains to take calculated decisions faster, also help avoid bringing many topics to TC.

@dougwilson
Copy link
Contributor

dougwilson commented Apr 15, 2020

Right, but if that is the case, then a project captain does not need to follow the guidelines, right? So GitHub already provides the guideline, and repos are welcome to follow those guidelines from Github, or choose not to. Am I missing something?

There are currently no guidelines for the organization as a whole to alter, which is why I was suggesting that it sounds like we would first need to make some and then work down to a specific guideline. If this is about a specific guideline in a specific repo, ideally it should just be brought up in that repo to let the repo deal with; bringing up here is a much wider discussion around a policy that doesn't exist at this level 😃

@gireeshpunathil
Copy link
Author

So GitHub already provides the guideline, and repos are welcome to follow those guidelines from Github, or choose not to. Am I missing something?

github does not provide guideline AFAIK, or at least the link I quoted does not say that. It provides a function, and it is up to projects to use that.

Even if github provides guidelines, it's guideline is different from project's guideline - w.r.t scope and visibility.

@gireeshpunathil
Copy link
Author

There are currently no guidelines for the organization as a whole to alter, which is why I was suggesting that it sounds like we would first need to make some and then work down to a specific guideline. If this is about a specific guideline in a specific repo, ideally it should just be brought up in that repo to let the repo deal with; bringing up here is a much wider discussion around a policy that doesn't exist at this level

Isn't collaborator's guide , for example, a set of guidelines that the entire organization follow?

@dougwilson
Copy link
Contributor

Even if github provides guidelines, it's guideline is different from project's guideline - w.r.t scope and visibility.

Right. And that's what I'm asking about here -- what is the goal for this discussion? If it is about adding that commit message trailer to commits in the express repo, that seems like an express repo discussion.

If the idea is that we should create commit message guidelines for the entire organization as a whole, then I think this issue has already gone down a much more narrow path, and we should open a new issue to discuss if that is something we even want to do, and then what said guidelines would even be; the issue seems to be a "cart before the horse" in that sense; there are no guidelines to alter currently.

Isn't collaborator's guide , for example, a set of guidelines that the entire organization follow?

No, it is just for the express repo.

@gireeshpunathil
Copy link
Author

@dougwilson - your own statements seem to contradict each other. For example:

there are no guidelines to alter currently.
(from above comment)

We do not include co-author fields, but we can if desired to amend our guidelines, in which this commit would wait for that to happen
(from expressjs/express#4210 (comment))

if there are no guidelines at the moment, which guidelines did you propose to amend? :)

@dougwilson
Copy link
Contributor

I believe you took that out of the context of the full sentence:

If the idea is that we should create commit message guidelines for the entire organization as a whole, then I think this issue has already gone down a much more narrow path, and we should open a new issue to discuss if that is something we even want to do, and then what said guidelines would even be; the issue seems to be a "cart before the horse" in that sense; there are no guidelines to alter currently.

You can see that "guidelines" at the end there refer to "guidelines for the entire organization" earlier in the sentence 😄 . The guidelines I'm referring to in the express repo are those for the express repo; that is why I'm saying that this is the wrong place to have a discussion about express repo-specific guidelines unless the idea is to begin an organization-wide guidelines proposal.

@gireeshpunathil
Copy link
Author

Unless I am missing something:

(to name a few)

in all these cases, the term express are aiming the entire express group of projects (express+jshttp+pillarjs) right? and those are all discussed here! And yet, why this is a wrong place for the current topic, and the discussion should go somewhere else?

@dougwilson
Copy link
Contributor

P.S. I'm heading to sleep now, as it is quite late where I am. I know you push on what is happening on 5.0, so here is a case where almost my entire open source time for the day was spent not going towards 5.0 work, haha--I think this has been a 2 hour discussion so far 😄 !

I hope I already made myself clear... I'm not sure what you are trying to get at... I'm the one landing commits in the expressjs/express repository, which follows how commits have always landed there to be uniform in the repository.

I have nothing against message trailers, but they are not used in that repository. Discussions about commit formats in a specific repository should be discussed.... in that repository, especially as we more towards project captains. Commit messages as a mundane day-to-day task in my opinion. If you think that it isn't, we should clarify that in the project captains proposal. If you think there should be an organization-wide commit guideline, then open a proposal for such a thing -- this issue is not that, mainly because of all the discussion that has already happened about the specifics around landing a commit in a specific repo -- all of which project captains calls out as something that shouldn't be escalating beyond the repo, in order to cut down the noise and the certain point.

IMO this entire discussion is fighting against the very thing I think you've been pushing: releasing 5.0. It has been pointed out that there are not a lot of folks, and that is true. The triagers have been great, but they are still not enough to cover even just the incoming issues still. I think we should perhaps just shelve this to regain the focus back onto 5.0...

@gireeshpunathil
Copy link
Author

I am sure you did not mean to close this?

it is so funny that the guidelines / ownership related discussion came up through a PR which in itself is supposed to address that. :)

I am fine to bring this up with captains when they are installed - with an added effort of talking to more people, individually.

But how, discussing it here, is proving to stall 5.0.0 work? I did not follow that part.

@dougwilson
Copy link
Contributor

I am sure you did not mean to close this?

I did, and I hope the action items are clear in the message above; if not let me know how I can clarify the next steps.

it is so funny that the guidelines / ownership related discussion came up through a PR which in itself is supposed to address that. :)

Right, and that PR IMO clarifiers that it is up to the folks in the repo itself -- who already said that it's not the way in which that repo works... I'm trying to highlight that bringing it up wider seems to contradict that very PR. If it doesn't, then the wording in the PR is not clear enough that commit messages do not fall under "repo ownership" and day-to-day tasks and we need to work to clarify that prior to merging

I am fine to bring this up with captains when they are installed - with an added effort of talking to more people, individually.

Right. You already did in the express repo and already got your answer -- the project captains already exist everywhere, as evidenced by the modules being worked on, committing to, publishing, etc. The document simply codifies it so as we continue to expand there is a name for it. All the repos not listed in that PR are currently myself--if you need that called out specifically, then list them all in the PR, but it was my understanding we were just not going to list out every single repo at the start.

But how, discussing it here, is proving to stall 5.0.0 work? I did not follow that part.

I'm not sure how much time you have on hand, but there is only so much time in the day. I wanted to get these PRs landed and some fixes up in router for 5.0, but instead all my attention for 75% of my night's open source time has been wrapped up in this single issue. Unless this issue is regarding the 5.0.0 process, that is just were that inference is coming from.

@gireeshpunathil
Copy link
Author

I wanted to get these PRs landed and some fixes up in router for 5.0, but instead all my attention for 75% of my night's open source time has been wrapped up in this single issue. Unless this issue is regarding the 5.0.0 process, that is just were that inference is coming from.

  • I am neither driving nor deciding other's quota of time on open source
  • It is always people's choice where they spend their time

Are you suggesting that I should not have opened this issue? Or something else?

@dougwilson
Copy link
Contributor

Are you suggesting that I should not have opened this issue? Or something else?

No, not at all. I was only saying that because I know you have asked a few times in TC meetings what the progress is, etc. and was simply trying to provide information if there is no meeting this week. If you would like me not to provide updates on 5.0 process unless specifically asked, I am happy to do so.

I stepped in here mainly because you were calling out the seemingly lack of participation from the TC, and so I'm trying to make sure at least I am providing the TC attention to items where they are needed and not letting things just sit for weeks, as that is also something we are trying to address (lack of movement on issues, PRs, etc.).

@gireeshpunathil
Copy link
Author

If you would like me not to provide updates on 5.0 process unless specifically asked, I am happy to do so.

I am sure you did not mean to say you are happy not to talk about 5.0.0!

I guess we are not going anywhere with this conversation. Yes, my only priority at the moment is to see 5.0.0 out. So request you that let us all:

  • take a step back on this, as it is already closed and I agreeing to your proposal to take anything further with individual repo captians;
  • refocus on 5.0.0 efforts!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants