Add commit co-authors support#1482
Add commit co-authors support#1482Byron merged 6 commits intogitpython-developers:mainfrom itsluketwist:add-co-authors
Conversation
… message for designated co-authors, include a simple test.
Byron
left a comment
There was a problem hiding this comment.
Thanks a lot, this looks good already!
I have some notes, but when clarified it should be ready for merging.
… the _from_string classmethod.
Byron
left a comment
There was a problem hiding this comment.
Thanks a lot!
The only remaining question pertains to the regex used, but once clarified this is ready to go.
Maybe you could also fix flake8 by ignoring this new check that makes CI fail, if you feel comfortable.
git/objects/commit.py
Outdated
| if self.message: | ||
| results = re.findall( | ||
| r"^Co-authored-by: ((?:\w|\-| ){0,38}) <(\S*)>$", | ||
| r"^Co-authored-by: ((?:\w|\-| ){0,38} <\S*>)$", |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
Thanks a lot for your contribution! |
Add co_authors property to the Commit object, which parses the commit message for designated co-authors, include a simple test.