Option for excluding rename commits from last update #148

skywarth · 2024-09-27T12:13:36Z

Discussed and proposed in #147

Did a couple of tests using one of my repositories where the aforementioned problem is present. Looking good so far.

This is just a draft PR, do not merge yet. Populate the TODO below if I'm missing something

TODO:

Manual test on an actual repository, via mkdocs serve
Automation tests defined in fixture
- Maybe define a new fixture for this case
Update the documentation to include description for this new option

…e date

timvink · 2024-09-27T12:25:41Z

I don't think we need an option for this. If there any use case where you would want this?

skywarth · 2024-09-27T12:50:01Z

I don't think we need an option for this. If there any use case where you would want this?

You mean this should be the default behavior, without an option?

I think the feature itself would be useful, because in knowledge-base and documentation projects there are often quite a lot of folder hierarchy changes, and if we consider these changes as 'last update' it could be misleading. Say for an API documentation; it was written in 2020 and never updated, which is deprecated today. And lately it was decided to be moved to another folder, and now it shows as it is updated recently, and people may think it is up-to-date, whereas it is not.

The reason to have it as option and not default behavior could be; it brings an additional overhead since it runs with --follow or follow=True and some people might not want such overhead which might cripple the performance a bit where it is imperative.

timvink · 2024-09-27T18:21:04Z

OK I had to dig back into the issue.

So if we use --follow, there is this really nasty bug that the date from git log --follow will be incorrect if there are any empty files committed in the git history (see https://blog.plover.com/prog/git-log-follow.html). Hence I was planning to make follow a plugin option, so users basically have to choose between incorrect creation/revision dates if there are file names, vs incorrect creation/revision dates if there are any empty files committed.

I do want to have sensible defaults. Here's my proposal:

We add a new plugin option follow, that defaults to True. This is a breaking change, because revision dates might changes (follow was already enabled for creation dates).
We add the diff filter r for revision dates. No plugin option, I think ignoring renames makes sense for both revision and creation dates. Does that diff filter = 'r'' need --follow? or can we just always add the diff filter? Maybe we can remove the conditional on follow like you have it now.
While we're making breaking changes, let's also add the --ignore-all-space and --ignore-blank-lines to the revision git log command. I think it's sensible, as those are not real edits. I can document it in the release notes.

Would you be willing to make the changes? I can take care of the documentation

timvink · 2024-09-27T18:24:24Z

src/mkdocs_git_revision_date_localized_plugin/util.py

                commit_timestamp = git.log(
-                    realpath, date="unix", format="%at", n=1, no_show_signature=True
+                    realpath, date="unix", format="%at", diff_filter=diff_filter_param, n=1, no_show_signature=True, follow=follow_param


Note that with follow enabled, you will get multiple commits, and you need to take the last one. See the logic used for the creation date.

Doesn't n=1 also achieve the same desired effect as selecting the last one using commit_timestamp = commit_timestamp.split()[-1] ?

I saw that and though maybe it is just awaiting refactoring.

If it doesn't achieve the same result, I can do it like the creation date.

skywarth · 2024-09-28T17:17:00Z

Sure @timvink, though I didn't fully wrap my head around it I'll try to implement these. If it gets way above my head I'll let you know 😄

…follow introduced, added additional parameter for revision changes, diff-filter "r" is added for both creation and revision commit histories

skywarth · 2024-09-29T21:53:19Z

Hi @timvink, just pushed the changes:

We add a new plugin option follow, that defaults to True. This is a breaking change, because revision dates might changes (follow was already enabled for creation dates).

We add the diff filter r for revision dates. No plugin option, I think ignoring renames makes sense for both revision and creation dates.

While we're making breaking changes, let's also add the --ignore-all-space and --ignore-blank-lines to the revision git log command. I think it's sensible, as those are not real edits. I can document it in the release notes.

About the "r" and --follow

"Does that diff filter = 'r'' need --follow? or can we just always add the diff filter? Maybe we can remove the conditional on follow like you have it now."

We can always add the diff-filter="r", when it is used along with --follow it essentially takes moving operations into account too. When it is used without the --follow it only tracks actual renames; renames that are limited to current directory, moving operations are not included since the file is not tracked/followed.

I got one question though; why did we add diff-filter "r" for creation dates as well? I was trying to come up with a case where it would make a difference but couldn't think of any, could you describe a case for it please?

And for the manual tests, here's the difference between master branch and this branch's outputs, based on one of my repositories:

problems.md
- master branch, (deployed): Revision date September 24, 2024. Creation date July 16, 2022.
- PR branch, version_history_follow_enabled: true: Revision date July 16, 2022. Creation date July 16, 2022.
- PR branch, version_history_follow_enabled: false: Revision date September 24, 2024. Creation date September 24, 2024.
great-advertisements.md
- master branch, (deployed): Revision date September 28, 2024. Creation date August 10, 2024.
- PR branch, version_history_follow_enabled: true: September 28, 2024. Creation date August 10, 2024.
- PR branch, version_history_follow_enabled: false: September 28, 2024. Creation date September 24, 2024.

Please note September 24, 2024 is the date I moved all files to another directory.

In conclusion: based on the test outputs, I think it achieves what we are looking for.

Let me know if you need additional changes or tests. I'm here to help.

Thank you

src/mkdocs_git_revision_date_localized_plugin/util.py

timvink · 2024-10-02T08:51:29Z

Nice work!

I got one question though; why did we add diff-filter "r" for creation dates as well? I was trying to come up with a case where it would make a difference but couldn't think of any, could you describe a case for it please?

I assume speed. Not sure anymore tbh.

Let me know if you need additional changes or tests. I'm here to help.

Cool!

can you rename version_history_follow_enabled to enable_git_follow
if you want, you can already update docs/options.md

Almost there!

skywarth · 2024-10-03T16:14:18Z

Certainly @timvink, I'm slightly busy nowadays but no worries I'll let you know once I update the PR, stay tuned. Thank you.

skywarth · 2024-10-12T22:39:14Z

Hi @timvink. Sorry for the delay, I was busy with relocating. Can you take a quick look and see if it's up to par?

timvink · 2024-10-17T06:13:37Z

Great stuff, thanks for all your effort to push this!

timvink · 2024-10-17T06:14:00Z

Will make a release this weekend

timvink · 2024-10-22T12:44:13Z

Released! https://github.com/timvink/mkdocs-git-revision-date-localized-plugin/releases/tag/v1.3.0

skywarth · 2024-10-22T12:48:27Z

Released! https://github.com/timvink/mkdocs-git-revision-date-localized-plugin/releases/tag/v1.3.0

Brilliant, cheers!

feat: option for excluding renames and move operations for last updat…

Loading
Loading status checks…

466e577

…e date

timvink reviewed Sep 27, 2024

View reviewed changes

skywarth marked this pull request as ready for review September 29, 2024 21:54

skywarth changed the title ~~Draft: Option for excluding rename commits from last update~~ Option for excluding rename commits from last update Sep 29, 2024

timvink reviewed Oct 2, 2024

View reviewed changes

src/mkdocs_git_revision_date_localized_plugin/util.py Show resolved Hide resolved

chore: rename option, add documentation

36b2159

skywarth requested a review from timvink October 17, 2024 00:26

timvink merged commit 9e064cd into timvink:master Oct 17, 2024

opalmedapps-bot mentioned this pull request Jan 28, 2025

chore(deps): update dependency mkdocs-git-revision-date-localized-plugin to v1.3.0 opalmedapps/docs#211

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Option for excluding rename commits from last update #148

Option for excluding rename commits from last update #148

skywarth commented Sep 27, 2024 •

edited

Loading

timvink commented Sep 27, 2024

skywarth commented Sep 27, 2024 •

edited

Loading

timvink commented Sep 27, 2024 •

edited

Loading

timvink Sep 27, 2024

skywarth Sep 29, 2024

skywarth commented Sep 28, 2024

skywarth commented Sep 29, 2024

timvink commented Oct 2, 2024 •

edited

Loading

skywarth commented Oct 3, 2024

skywarth commented Oct 12, 2024

timvink commented Oct 17, 2024

timvink commented Oct 17, 2024

timvink commented Oct 22, 2024

skywarth commented Oct 22, 2024

Option for excluding rename commits from last update #148

Option for excluding rename commits from last update #148

Conversation

skywarth commented Sep 27, 2024 • edited Loading

timvink commented Sep 27, 2024

skywarth commented Sep 27, 2024 • edited Loading

timvink commented Sep 27, 2024 • edited Loading

timvink Sep 27, 2024

Choose a reason for hiding this comment

skywarth Sep 29, 2024

Choose a reason for hiding this comment

skywarth commented Sep 28, 2024

skywarth commented Sep 29, 2024

timvink commented Oct 2, 2024 • edited Loading

skywarth commented Oct 3, 2024

skywarth commented Oct 12, 2024

timvink commented Oct 17, 2024

timvink commented Oct 17, 2024

timvink commented Oct 22, 2024

skywarth commented Oct 22, 2024

skywarth commented Sep 27, 2024 •

edited

Loading

skywarth commented Sep 27, 2024 •

edited

Loading

timvink commented Sep 27, 2024 •

edited

Loading

timvink commented Oct 2, 2024 •

edited

Loading