-
Notifications
You must be signed in to change notification settings - Fork 140
fix: push parsing drops commit messages unless they end in a newline #976
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
Conversation
✅ Deploy Preview for endearing-brigadeiros-63f9d0 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #976 +/- ##
==========================================
+ Coverage 48.70% 53.80% +5.09%
==========================================
Files 53 53
Lines 2166 2171 +5
Branches 242 244 +2
==========================================
+ Hits 1055 1168 +113
+ Misses 1074 952 -122
- Partials 37 51 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Fixed the cypress test but am waiting on a review (in git proxy), will update monday with working cypress tests |
Code coverage is probably as good as I can get it - more to come on DB code coverage in #979 |
@JamieSlome @grovesy @coopernetes This is ready for a review |
If I recall, the newline at the end of each commit message came in every time, so was reliably always a way of delimiting?
We had two commits, from the same developer, that came in without the terminating newline (as per example in the issue) so I suspect it boils down to whether the git client adds the newline. Unfortunately, they couldn't remember which client they used for that commit (git command line, VS code or GitHub desktop).
The lack of the newline clearly doesn't break git itself so I guess it can't be relied on...
As for a test... I'll have another look. At present I think parsePush is untested so we'll probably need to capture some raw data to can for the test...
|
tests on the way - edit: tests for parsePush added |
Tests added based on conventional commit messages with a newline at the end. I get zlib errors (`Error: incorrect header check') as it processes the compressed content, I suspect it always throws these. Doesn't seem to cause any issue with the parsing however... |
One more test coming (took me a while to find and harvest data from a commit with no newline at the end of the message)... Will push tomorrow |
@JamieSlome added the final test. @hemantsharma90 tracked the commits with no newline back to the IntelliJ git client. They do appear to be valid, and parse correctly after this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🍰
resolves #971
Adjusts push parsing to not require a newline at the end commit messages in order to capture them.
Also adjusts CLI tests to clean up the rows they create, so that they can be run multiple times without the DB files needing to be reset.