Skip to content
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

Attempt to extract date from only the last line of git log output #125

Closed
wants to merge 1 commit into from

Conversation

yuha0
Copy link

@yuha0 yuha0 commented Jun 24, 2022

Previously, the code assumes the output of git log --format="%at" only
contains commit timestamp. But this is not true.

--format only controls a portion of the output. git log command can
output additional information thats not controlled by format. e.g.: When
showSignature is set, it will always display commit signature info.

This commit attempts to locate timestamps only from the very last line
of git log output.

Fixes #124

@yuha0
Copy link
Author

yuha0 commented Jun 24, 2022

I am not super familiar with all the possible options in git that may affect the output of git log. There could have been other cases I haven't thought about. However, limiting the timestamp extraction to only the last line of git log output fixed the issue for me at least. Please let me know if you have suggestions!

@codecov
Copy link

codecov bot commented Jun 24, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.21%. Comparing base (3dabb2c) to head (293a902).

Current head 293a902 differs from pull request most recent head e950034

Please upload reports for the commit e950034 to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #125      +/-   ##
==========================================
- Coverage   85.04%   81.21%   -3.84%     
==========================================
  Files          10        5       -5     
  Lines         555      330     -225     
  Branches      117       76      -41     
==========================================
- Hits          472      268     -204     
+ Misses         53       40      -13     
+ Partials       30       22       -8     
Flag Coverage Δ
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
mkdocs_rss_plugin/util.py 71.42% <ø> (-3.78%) ⬇️

... and 10 files with indirect coverage changes

@Guts
Copy link
Owner

Guts commented Jun 30, 2022

It looks good to me but maybe an unit test would be better, to compare result between with/out the --show-signature option. What do you think?

@yuha0
Copy link
Author

yuha0 commented Jul 6, 2022

@Guts
Thanks for the suggestion. I think that's a great idea. Looks like the lines I touched are part of a very long method, get_file_dates, which attempts to extract dates in various ways. What do you think if we refactor it a bit and move the git log extraction logic into a dedicated string parsing method? Then I can mock some git log outputs to test just that method.

@yuha0 yuha0 force-pushed the git-log-ignore-signature branch from 0c944b5 to 293a902 Compare July 6, 2022 00:54
@Guts
Copy link
Owner

Guts commented Jul 6, 2022

What do you think if we refactor it a bit and move the git log extraction logic into a dedicated string parsing method? Then I can mock some git log outputs to test just that method.

Good idea! Please, make yourself comfortable, go ahead!

@Guts
Copy link
Owner

Guts commented Aug 29, 2022

Hi @yuha0!

Do you need some help or review to achieve this?

@yuha0
Copy link
Author

yuha0 commented Oct 3, 2022

@Guts Sorry about the delay, It's been busy at work. Also, for some reason I cannot reproduce it on my personal device... I will need to investigate a bit more

@Guts
Copy link
Owner

Guts commented Oct 4, 2022

@yuha0 no worries, take the time you have. I just needed to know if it's still an active work here or if it's stale.

Can you set this PR to draft status in the meanwhile?

@yuha0 yuha0 marked this pull request as draft October 5, 2022 06:18
Previously, the code assumes the output of `git log --format="%at"` only
contains commit timestamp. But this is not true.

`--format` only controls a portion of the output. `git log` command can
output additional information thats not controlled by format. e.g.: When
`showSignature` is set, it will always display commit signature info.

This commit attempts to locate timestamps only from the very last line
of `git log` output.
@Guts Guts force-pushed the git-log-ignore-signature branch from 293a902 to e950034 Compare June 10, 2024 11:23
Copy link

sonarcloud bot commented Jun 10, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Guts
Copy link
Owner

Guts commented Jun 10, 2024

Hey @yuha0,

Any chance to see this work achieved?
I close here since it's has been 2 years without any push (except rebase), but don't hesitate to reopen.

@Guts Guts closed this Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

date detection from "git log" is broken when git client is configured with "--show-signature"
2 participants