- Issue created by @dww
- 🇺🇸United States dww
Re:
Not sure the cleanest way to handle "Committed-by", but maybe that "needs" to be here, too?
.
I guess we'll always have the formal Git
Author
header for that, so it doesn't need to be duplicated in the commit message body if we don't want to mess with it. So long as core committers never push commits with --author shenanigans, we're fine. Although, bummer, I just did a grep of the core commit log:git log egrep 'Author: ' | sort | uniq -c | sort -r
The results are attached. There are approximately 30 that I recognize as core committers, and maybe 150 unique authors that must have been "forged" on commit before pushing. I think d.o separately had/has a notion of who pushes for credit and issue updates, not just the
--author
value. So probably we shouldn't rely on Author, and it'd perhaps be safer to stuff "Committed-by" (or perhaps "Merged-by"?) into the footer, too? - 🇪🇸Spain fjgarlin
I had a long standing
@todo
in the comment about this: https://git.drupalcode.org/project/contribution_records/-/blob/1.0.x/js/...Ideally, each row would default to "Authored-by" for anyone who pushed commits, and "Reviewed-by" for everyone else. If a reporter hasn't otherwise commented or pushed a commit, default to "Reported-by"? ... git log egrep 'Author: ' | sort | uniq -c | sort -r
We are going into deep analysis of the code, the commits, etc. This feels like overkill and it will be prone to errors and complex code.
Sometimes it feels that if we add more options to the UI it will take longer than just changing the suggested message once copied.In any case, the idea/suggestion of adding a dropdown at the end of each row, where the options will be: author, reviewer, reporter (, committed) is good. As mentioned, we can set them all to "author" and then leave the person crafting the commit message change those if desired.
Doing the initial code, CSS and JS might not be too difficult, if things don't need to persist.
Up until here it would be relatively easy and totally doable. But things might get a bit more complicated and uneccessarily complex if we want to save that information. Do we really need to keep that level of detail in the database?
Also, do we want to build tooling to go through commit history of MRs to determine the role in the footer of the commit message?
- 🇺🇸United States drumm NY, US
A big concern I have with the credit system is maintainers needing to spend time doing crediting paperwork. The commit trailers are not used in any crediting on Drupal.org, they are only for people who read the commit history. I know core maintainers already take their crediting task seriously and spend time granting crediting as fairly as possible.
Even if we can do a decent job of automating authored/reviewed/reported/etc-by, are the extra choices for maintainers to add more detail just to a commit message worth their time? Maintainers who do want to add the extra detail can edit the commit message after copying it.
- 🇺🇸United States cmlara
Just was encountering this as a contrib maintainer where it was suggesting author status for me when I haven't done any code (and would not be entitled to an author statues).
are the extra choices for maintainers to add more detail just to a commit message worth their time? Maintainers who do want to add the extra detail can edit the commit message after copying it.
Having the UI would reduce the time maintainers have to spend adding the extra detail, a few hours work to save countless hours of maintainer time per year compounded over the next decade.
Core included this in their commit standard, I would assume Infra would want to implement this for them (otherwise they might as well just use the GitLab UI).
The commit trailers are not used in any crediting on Drupal.org, they are only for people who read the commit history.
Anyone listed as an author would be generally presumed to potentially hold copyright to the code and joint liability for any misappropriated code, while reviewers may have not blocked the code from enter the repo they can reasonably assert that they were not involved in the obtaining of said code.
This can lead into even more interesting territory when we discuss issues where a user comments that they object to code, if the code latter causes issues (site down/etc) including them as an 'author' could be possibly seen as libel
As an infrequent contributor to core: I absolute do not want my name listed as an author if all I did was endorse the concept (or worse, if I objected to the concept).
A big concern I have with the credit system is maintainers needing to spend time doing crediting paperwork.
Unfortunately our time to resolve that was before this was adopted as a standard, now that its standard we just have to deal with it.
- 🇳🇿New Zealand quietone
A big concern I have with the credit system is maintainers needing to spend time doing crediting paperwork.
Same here. I usually like anything that allows for more details but not this. Adding credit is already a challenge and time consuming task for core issues. This makes that more difficult. On the face of it the 3 roles seem to be self-explanatory but there will be edge cases and debate on those. And it can also be a reason for more requests to review issue credit because the role is not as expected.
In the end, I am not keen on providing this.
- 🇺🇸United States dww
If folks don’t want this to reflect reality, then I’m opposed to anyone being called “Authored-by”. If we’re only allowing a single “role” it should be something generic and bland like
“Assisted-by” or “Contribution-by” or something. But calling all creditable participants an author is really problematic IMHO.In terms of time spent: if the credit UI saved any state, subsystem maintainers could help craft the right commit message during RTBC.
Fallback would be to draft the right commit message as the body in the GitLab MR description. I do that on some issues, but core committers never use that.
Also, the heuristics I proposed above would cover 95% of the work, I think. If you push any commits, you’re a “Authored-by” user, otherwise, “Reviewed-by”.
Finally, I’m strongly in favor of spending a little development time on this feature so that it’s easier for core maintainers to get this right. We can’t seriously claim that a few hours of DA dev time (once) are more precious than dozens / hundreds of hours of committer time each year doing it manually.
- 🇺🇸United States cmlara
The DA/Core may be abandoning the roles classification.
Infra pushed 📌 Simplify commit message Active changing it to just
By: foo
.Core is discussing policy in ✨ Simplify agreed commit format Active .
Not closing this issue as outdated as it could be considered a followup request to restore the feature.