Add ignored commits feature #114
Conversation
|
|
||
| ## `ignored_commits_file` | ||
|
|
||
| Default is `None`. When specified, contains a file that contains a list of commit hashes to ignore |
There was a problem hiding this comment.
'When specified, contains a file that contains a list' -- That sentence is a bit weird. Explain it needs a path, and relative to the projects root (mkdocs.yml file?).
| ## `ignored_commits_file` | ||
|
|
||
| Default is `None`. When specified, contains a file that contains a list of commit hashes to ignore | ||
| when determining the most recent updated time. The format of the file is the same as the format of |
There was a problem hiding this comment.
Sometime we use git blame to determine the creation date also. So replace determining 'the most recent updated time' with 'the revision date'.
| @@ -117,6 +120,7 @@ def get_git_commit_timestamp( | |||
| realpath = os.path.realpath(path) | |||
| git = self._get_repo(realpath) | |||
|
|
|||
| # Ignored commits are only considered for the most recent update, not for creation | |||
There was a problem hiding this comment.
With 150k downloads per month, someone is bound to stumble on this. It should ignore those commits everywhere.
Best to refactor, using a separate function that takes in a list of commits and return the last or first one that is not in the ignored commits list. You can use that function for both creation and most recent update.
We also need a proper fallback inside that function. What if all commits are ignored (f.e. because there is only 1)? You would think that never happens, but it could happen for new repos. Or, some users, f.e. when using CI, use a shallow clone, that only retrieves the last commit. If that commits happens to be an ignored commit -- there are no more commits left to determine the build date.
Probably the best fallback for that scenario is to then use ignored commits anyway. We'd need to document that. And we probably want to inform the user that that happened. It should log a warning when the strict option is True.
| Whitespace, blanklines and comments starting with # are all ignored. | ||
| """ | ||
| result = [] | ||
| with open(filename, "rt") as f: |
There was a problem hiding this comment.
perhaps we should specify UTF-8 encoding. This is standard for mkdocs. See f.e. https://github.com/mkdocs/mkdocs/blob/df3739d51903ab56771ac071a05b5aa9cdf9e129/mkdocs/commands/new.py#L43
There was a problem hiding this comment.
Also, what if this file is not found, because a user specified the wrong path? Can we give a helpful error, showing with path was tried and which parameter in mkdocs.yml should be changed?
| @@ -672,3 +677,37 @@ def test_mkdocs_genfiles_plugin(tmp_path): | |||
| validate_build( | |||
| testproject_path, plugin_config | |||
| ) | |||
|
|
|||
There was a problem hiding this comment.
well done on writing these unit tests!
|
Thanks! Well done on figuring this one out. Too bad we couldn't find something more elegant with using git internals like what is possible for I have some comments, and there are some failing unit tests. Please have a look :) |
|
I believe that the tests failed because of this codecov issue: https://about.codecov.io/blog/message-regarding-the-pypi-package/ Could you please re-run them? Then I can address any test failures along with the refactoring you requested all at once. |
This addresses #113 by adding a new option "ignored_commits_file" that points to a file to contain commit hashes in the same format as
blame.ignoreRevsFilethat will be ignored during calculation of the most recent update. To the best of my knowledge, it is not possible to askgit logto ignore commits, so this must be done by the plugin.I've added a basic unit test for the file parser, and an integration test to validate the commit ignoring behavior.
I've tested this on my own repository and verified that it behaves as expected.
pyflakes shows no new issues. pytest shows no changed lines without coverage.