Skip to content

Add commit co-authors support #1482

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

Merged
merged 6 commits into from
Aug 25, 2022
Merged

Add commit co-authors support #1482

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
146cbda
Add co_authors property to the Commit object, which parses the commit…
itsluketwist Aug 22, 2022
3cb7ecf
Add malformed co-authors to the test, to check they aren't detected w…
itsluketwist Aug 23, 2022
c2fd97e
Update regex to extract the author string, and create the Actor using…
itsluketwist Aug 23, 2022
09f8a1b
Use the same regex as the Actor class when determining co-authors.
itsluketwist Aug 24, 2022
4c460a3
Ignore flake8 error.
itsluketwist Aug 24, 2022
72cf71c
Allow failure of that one test on cygwin
Byron Aug 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions git/objects/commit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,11 +752,11 @@ def co_authors(self) -> List[Actor]:

if self.message:
results = re.findall(
r"^Co-authored-by: ((?:\w|\-| ){0,38}) <(\S*)>$",
r"^Co-authored-by: ((?:\w|\-| ){0,38} <\S*>)$",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, can you explain where this regex is coming from? Earlier I tried to argue that the one used by Actor could be used here. It's much more open than this, but I can be swayed if there is a specification of the Co-authored-by: field that limits it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my bad! Regarding the name part, this is on me, I used a regex to match against the GitHub username specs, but this is just the account name, so a more generic one is necessary - switched to match the regex used in Actor (with the extra Co-authored-by: text and start/end line checks).

Also added a line to ignore the flake8 check - not used flake8 but I think this should be right.

self.message,
re.MULTILINE,
)
for author in results:
co_authors.append(Actor(*author))
for author_string in results:
co_authors.append(Actor._from_string(author_string))

return co_authors