Dexmaster
commented
3 years ago Hello @aeiras and @scottmarlow.
We found that issue 4-5 month ago when doing 9.1 changes to the tool. And added so all authors and co-authors (up to 10) for commits would be added to the list, even if their changes already are not there commits should be still in a tree. (these lines are where fix is)
If PR was properly merged/squashed those users will appear in a list because they will be in the authored list.
But this case is completely different issue, if you force remove commit from tree or do a soft-reset squash without adding authors for previous commits to footer as:
fix(JK-001): Squash commit title
Commit body
Co-Authored-By: John Doe <a@example.com>
Co-Authored-By: Monty Python <b@example.com>It will not matter, it's an issue of removing commit history at that point. Good indication that history was tampered with is, PR merge commit should be in the commit list, if you remove some file it will just be another commit. If you revert PR merge it should be also another commit...
The only way I see this could happen is if it was force pushed with overwriting old commit history.
That's why master/main branches have force push protection by default, and it should be on any branches used for stable development.
Conclusion: Tool for committers is pulling all PR and commit authors (with commit co-authors). The only issue is if commit is improperly manually squashed (without co-author contribution), or commit history was forcibly rewritten, but it's more a question of development best practices and contribution, it is hard to fix through just having raw data. (there is no even indication it even existed in that branch)
I wrote a small test for this case with the same graphql code as we use in commit getting bash script. (you can test it on https://docs.github.com/en/graphql/overview/explorer) No @cesarhernandezgt there, but I also don't see that PR merge commit even squashed (or revert of it) in commit list...
query {
repository(name:"jakartaee-tck" owner:"scottmarlow") {
ref(qualifiedName: "tckrefactor_marlow") {
target {
... on Commit {
history(first: 100, since: "2021-08-23T00:00:00+00:00") {
pageInfo {
hasNextPage
endCursor
}
edges {
node {
commitUrl
authors(first:10) {
nodes {
... on GitActor {
user {
login,
name
}
}
}
}
}
}
}
}
}
}
}
}P.S. PR with multi-author commits fix for a bash tool is https://github.com/jakartaee/jakarta.ee/pull/1194
scottmarlow
During the Sept 8th TCK call, we discussed squash vs not squash and its impact on the Jakarta Releases Contributors' cards history that populates the release page cards.
Thanks to @scottmarlow, we know that TCK items might be impacted by the current clean up. [1] https://github.com/scottmarlow/jakartaee-tck/pull/3 [2] https://github.com/scottmarlow/jakartaee-tck/tree/tckrefactor_marlow_beforesquash [3] https://github.com/scottmarlow/jakartaee-tck/tree/tckrefactor_marlow
Andrii, the Triber behind the development of the contributor's tool, found out the follow Conclusion: Squash is not optimal to commit history with reference to the stats used for the tool.
Quoting Andrii: "the Tool for committers is pulling all PR and commit authors with co-authors. The only issue is if commit is improperly manually squashed (without co-author contribution), or commit history was forcibly rewritten, but it's more a question of development best practices and contribution, it is hard to fix through just having raw data. (there is no even indication it even existed in that branch)"
@Dexmaster, you are most welcomed to expand your findings and Q&A as needed.
Scott, questions, shoot!
Thanks for enabling this discussion :)