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

Updated git-committers to 1.1.0 #4303

Merged
merged 6 commits into from Oct 1, 2022
Merged

Conversation

ojacques
Copy link
Sponsor Contributor

@ojacques ojacques commented Aug 31, 2022

This PR updates the mkdocs-git-committers-2 plugin from 0.4.3 to 1.1.0.
Version 1.1.0 is a complete re-write. Please check the link to see what has changed.

The results will now be reliable, and this does not require a GitHub personal access token anymore.

I tested it successfully against the insiders repository.

@squidfunk
Copy link
Owner

Thanks! There are still some rough edges, see ojacques/mkdocs-git-committers-plugin-2#17

@ojacques
Copy link
Sponsor Contributor Author

ojacques commented Sep 1, 2022

@squidfunk - should be good now with 1.0.1.

@ojacques ojacques changed the title Updated git-committers to 1.0.0 Updated git-committers to 1.0.2 Sep 4, 2022
@squidfunk
Copy link
Owner

Hmm, does caching still work? Do I need to configure something now? I'm seeing repeated calls in subsequent builds.

@squidfunk
Copy link
Owner

I'm seeing this every time:

INFO     -  Building documentation...
WARNING  -  Config value: 'plugins'. Warning: ('token', 'Unrecognised configuration name: token')
INFO     -  git-committers plugin ENABLED
WARNING  -  A 'dirty' build is being performed, this will likely lead to inaccurate navigation and other links within your site. This option is designed for site development purposes only.
INFO     -  Fetching contributors for docs/index.md
INFO     -  Fetching contributors for docs/alternatives.md
INFO     -  Fetching contributors for docs/browser-support.md
INFO     -  Fetching contributors for docs/creating-your-site.md
INFO     -  Fetching contributors for docs/customization.md
INFO     -  Fetching contributors for docs/getting-started.md
INFO     -  Fetching contributors for docs/license.md
INFO     -  Fetching contributors for docs/philosophy.md
INFO     -  Fetching contributors for docs/publishing-your-site.md
INFO     -  Fetching contributors for docs/upgrade.md
INFO     -  Fetching contributors for docs/blog/index.md
INFO     -  Fetching contributors for docs/blog/2021/excluding-content-from-search.md
INFO     -  Fetching contributors for docs/blog/2021/search-better-faster-smaller.md
INFO     -  Fetching contributors for docs/blog/2021/the-past-present-and-future.md
INFO     -  Fetching contributors for docs/blog/2022/chinese-search-support.md
INFO     -  Fetching contributors for docs/changelog/index.md
INFO     -  Fetching contributors for docs/insiders/index.md
INFO     -  Fetching contributors for docs/insiders/changelog.md
INFO     -  Fetching contributors for docs/insiders/getting-started.md
INFO     -  Fetching contributors for docs/reference/index.md
INFO     -  Fetching contributors for docs/reference/admonitions.md
INFO     -  Fetching contributors for docs/reference/annotations.md
INFO     -  Fetching contributors for docs/reference/buttons.md
INFO     -  Fetching contributors for docs/reference/code-blocks.md
INFO     -  Fetching contributors for docs/reference/content-tabs.md
INFO     -  Fetching contributors for docs/reference/data-tables.md
INFO     -  Fetching contributors for docs/reference/diagrams.md
INFO     -  Fetching contributors for docs/reference/footnotes.md
INFO     -  Fetching contributors for docs/reference/formatting.md
INFO     -  Fetching contributors for docs/reference/grids.md
INFO     -  Fetching contributors for docs/reference/icons-emojis.md
INFO     -  Fetching contributors for docs/reference/images.md
INFO     -  Fetching contributors for docs/reference/lists.md
INFO     -  Fetching contributors for docs/reference/mathjax.md
INFO     -  Fetching contributors for docs/reference/tooltips.md
INFO     -  Fetching contributors for docs/setup/adding-a-comment-system.md
INFO     -  Fetching contributors for docs/setup/adding-a-git-repository.md
INFO     -  Fetching contributors for docs/setup/building-for-offline-usage.md
INFO     -  Fetching contributors for docs/setup/changing-the-colors.md
INFO     -  Fetching contributors for docs/setup/changing-the-fonts.md
INFO     -  Fetching contributors for docs/setup/changing-the-language.md
INFO     -  Fetching contributors for docs/setup/changing-the-logo-and-icons.md
INFO     -  Fetching contributors for docs/setup/ensuring-data-privacy.md
INFO     -  Fetching contributors for docs/setup/setting-up-navigation.md
INFO     -  Fetching contributors for docs/setup/setting-up-site-analytics.md
INFO     -  Fetching contributors for docs/setup/setting-up-site-search.md
INFO     -  Fetching contributors for docs/setup/setting-up-social-cards.md
INFO     -  Fetching contributors for docs/setup/setting-up-tags.md
INFO     -  Fetching contributors for docs/setup/setting-up-the-footer.md
INFO     -  Fetching contributors for docs/setup/setting-up-the-header.md
INFO     -  Fetching contributors for docs/setup/setting-up-versioning.md
INFO     -  Fetching contributors for docs/setup/extensions/index.md
INFO     -  Fetching contributors for docs/setup/extensions/python-markdown-extensions.md
INFO     -  Fetching contributors for docs/setup/extensions/python-markdown.md
INFO     -  Documentation built in 44.22 seconds

@ojacques
Copy link
Sponsor Contributor Author

ojacques commented Sep 7, 2022

The way to get contributors has completely changed. The plugin does not anymore try to guess / match local git commits with users in github, but rather gets the information directly from GitHub, for each page.
As such, caching of github users is no more needed.

The drawback is that we have an http call now for each page. The benefit is that contributors are now accurate (at least match GitHub's perspective).

Thinking of the caching aspect a bit further, it is possible to cache the results - but I am not sure still when to invalidate the cache for each page. Using the git log seems to be a way.

Let me ask: do you have a big impact on your builds?

@squidfunk
Copy link
Owner

Yes, I now need to disable the plugin each time I work on the project, because the build time went from 9s to 44s. I need to restart quite often, which is why I feel that removing caching is a step back.

Copy link
Sponsor Contributor Author

@ojacques ojacques left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump to git-committers 1.1.0 to benefit from cache

@ojacques ojacques changed the title Updated git-committers to 1.0.2 Updated git-committers to 1.1.0 Sep 7, 2022
@ojacques
Copy link
Sponsor Contributor Author

ojacques commented Sep 7, 2022

I created a new version - 1.1.0 which re-introduces a cache, per page. In my tests:

  • Building mkdocs material documentation without the plugin: 11s
  • Building mkdocs material documentation with the plugin, no cache: 37s
  • Building mkdocs material documentation with the plugin, 3 pages not in cache: 14s

Note: the message on the token is because you kept token as an attribute in mkdocs.yml. This token is not needed anymore, as the plugin does not use the GitHub API.

Let me know how it goes. Sorry for back & forth!

@squidfunk
Copy link
Owner

Thanks! I'll check it out this weekend. Yes, I know about the token setting, but I have pinned the plugin to <1 because of the missing dependency that broke the build earlier. Need to find some time to do the upgrade.

@squidfunk
Copy link
Owner

This took some time, sorry for that – I had to fix a lot of bugs on the blog plugin 😅 If I update to 1.1.0 on Insiders, I'm now greeted with the following error message:

ERROR    -  Error building page 'blog/posts/blog-support-just-landed.md': 'docs/blog/posts/blog-support-just-landed.md'
Traceback (most recent call last):
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/bin/mkdocs", line 8, in <module>
    sys.exit(cli())
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/mkdocs/__main__.py", line 181, in serve_command
    serve.serve(dev_addr=dev_addr, livereload=livereload, watch=watch, **kwargs)
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/mkdocs/commands/serve.py", line 63, in serve
    config = builder()
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/mkdocs/commands/serve.py", line 58, in builder
    build(config, live_server=live_server, dirty=dirty)
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/mkdocs/commands/build.py", line 314, in build
    _build_page(file.page, config, doc_files, nav, env, dirty)
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/mkdocs/commands/build.py", line 212, in _build_page
    context = config['plugins'].run_event(
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/mkdocs/plugins.py", line 102, in run_event
    result = method(item, **kwargs)
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/mkdocs_git_committers_plugin_2/plugin.py", line 112, in on_page_context
    authors, last_commit_date = self.list_contributors(git_path)
  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/mkdocs_git_committers_plugin_2/plugin.py", line 74, in list_contributors
    return self.cache_page_authors[path]['authors'], self.cache_page_authors[path]['last_commit_date']
KeyError: 'docs/blog/posts/blog-support-just-landed.md'

Any thoughts why this might be happening? If I disable the plugin, all works fine. Also, 0.4.4 still works without problems.

@jasonren0403
Copy link
Contributor

jasonren0403 commented Sep 19, 2022

Sorry to jump in, is the new version of git-committers returning correct commiters of the document? I found the current version somehow buggy about displaying contributors.
Let's see contributions to this passage
image
image
The two profiles' link is as follows, notice they have the same Full Name field
https://github.com/jasonren0403
https://github.com/jason718

@squidfunk
Copy link
Owner

@jasonren0403 this might be related to the fact that we're using 0.4.4 because of the errors reported in my last comment. It might be fixed in 1.1.0, but maybe @ojacques can clarify.

@ojacques
Copy link
Sponsor Contributor Author

ojacques commented Sep 19, 2022

If I update to 1.1.0 on Insiders, I'm now greeted with the following error message:

  File "/Users/squidfunk/Desktop/Projects/Personal/Sources/mkdocs-material-insiders/venv/lib/python3.8/site-packages/mkdocs_git_committers_plugin_2/plugin.py", line 74, in list_contributors
    return self.cache_page_authors[path]['authors'], self.cache_page_authors[path]['last_commit_date']
KeyError: 'docs/blog/posts/blog-support-just-landed.md'

Any thoughts why this might be happening? If I disable the plugin, all works fine. Also, 0.4.4 still works without problems.

@squidfunk - I found the issue, this is fixed in 1.1.1. Workaround is to delete the cache file.

@ojacques
Copy link
Sponsor Contributor Author

Sorry to jump in, is the new version of git-committers returning correct commiters of the document?

@jasonren0403 - yes. With version 1.x.x, the contributors you see on GitHub are the contributors you will get on the documentation. Previously, the plugin was trying to match local git committers with the one on GitHub, which could lead to false matches. The new version fetches directly the info from each file on GitHub.

@squidfunk
Copy link
Owner

Thanks! Now, all avatar URLs are #? If I check authors.json, I only see #:

{
  "squidfunk": {
    "login": "squidfunk", 
    "name": "squidfunk", 
    "url": "#", 
    "avatar": "https://www.gravatar.com/avatar/174f624169542490d134f6ebfa6989f7?d=identicon"
  }
}

@ojacques
Copy link
Sponsor Contributor Author

Thanks! Now, all avatar URLs are #? If I check authors.json, I only see #:

In the new version, the cache is named page-authors.json. For me, it looks correct with url being populated:

{
    "cache_date": "2022-09-28",
    "page_authors": {
        "docs/index.md": {
            "last_commit_date": "2020-03-26",
            "authors": [
                {
                    "login": "squidfunk",
                    "name": "squidfunk",
                    "url": "https://github.com/squidfunk",
                    "avatar": "https://avatars.githubusercontent.com/u/932156"
                },
                {
                    "login": "coliff",
                    "name": "coliff",
                    "url": "https://github.com/coliff",
                    "avatar": "https://avatars.githubusercontent.com/u/1212885"
                },
                {
                    "login": "Eskumu",
                    "name": "Eskumu",
                    "url": "https://github.com/Eskumu",
                    "avatar": "https://avatars.githubusercontent.com/u/21247241"
                },
                {
                    "login": "edmorley",
                    "name": "edmorley",
                    "url": "https://github.com/edmorley",
                    "avatar": "https://avatars.githubusercontent.com/u/501702"
                }
            ]
        },
        "docs/alternatives.md": {
            "last_commit_date": "2022-09-14",
            "authors": [
                {
                    "login": "squidfunk",
                    "name": "squidfunk",
                    "url": "https://github.com/squidfunk",
                    "avatar": "https://avatars.githubusercontent.com/u/932156"
                },
                {
                    "login": "pawamoy",
                    "name": "pawamoy",
                    "url": "https://github.com/pawamoy",
                    "avatar": "https://avatars.githubusercontent.com/u/3999221"
                }
            ]
        }
    }

For the names, the plugin now uses the login name. Getting the full name will require an API call which I don't think is necessary.

@squidfunk
Copy link
Owner

I can confirm that it works now. Thanks for your patience and for your work on this great plugin!

@squidfunk squidfunk merged commit cec89f0 into squidfunk:master Oct 1, 2022
@ojacques
Copy link
Sponsor Contributor Author

ojacques commented Oct 1, 2022

And thanks for your sponsorship!

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.

None yet

3 participants