-
Notifications
You must be signed in to change notification settings - Fork 7
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
Cache data fetched from GitHub between builds #12
Comments
@squidfunk, I hate to hijack this issue, but performance is one big reason I had to fork this project to create my own (I really dislike forking, but had to). https://github.com/ojacques/mkdocs-git-committers-plugin-2 I have happily used it with mkdocs-material, although an old version. Happy to help. Also happy to get my changes in this upstream repo. |
@ojacques thanks! I've test-driven your project, but unfortuntely it produced inconsistent results for me. For example, my account (@squidfunk) was not correctly determined (login was empty). Maybe you can try it on the Material for MkDocs repository and track it down. I'm currently working on a tighter GitHub integration for this plugin and the |
Yes, I'll have a look. If this could be part of material proper, it would help me a ton. The short video is awesome (although I loved having contributors on top and not at the bottom). |
@squidfunk - after testing against mkdocs-material, I pushed a new version (0.4.0), which:
It does not cache between builds though, but makes sure to only query info for a user once. To be precise, it will try first to get the info with the email then the name if not found, then the GitHub username if still not found. Caching between builds is not there yet, but I can work on it by saving the dictionary file at the end of site generation and attempting to load it if it exists at the beginning. |
Done in 0.4.1. Although I'm not sure this is the best way/place to save this cache file. There is a risk for it to be committed if not added as part of But hey, |
Awesome, thanks for investing the time! I'll check it out the next time I work on this enhancement. I just put files to cache into a folder called
You will be able to easily change the location by overriding the |
I can confirm that the build time went waaaaay down, awesome job 🚀 I'll now consider it safe to recommend this plugin! The improved git plugin integration will be released in the coming weeks, I'll put a link to your plugin in our documentation. One more thing: could we maybe make the cache folder configurable? I'd like to keep my @byrnereese sorry for hi-jacking this issue, we should really have discussed this over at the fork. Edit: Material for MkDocs' plugins will now write their caches into subfolders. Maybe this is also something to consider for the
We recommend caching on our docs, which makes use of the |
Why don't you issue a PR so we can stay in sync. No need to fork if we are all aligned. |
@byrnereese Absolutely. I will issue a PR against this repo. It may be big / hard to review. I apologize in advance. |
@byrnereese your plugin looks a little unmaintained, as there are three open issues without any answers and there's not much commit activity. I guess this is also why the fork exists. If we could gravitate towards one solution, that would be awesome, but we'd need to be able to push out bug fixes fast. I'd expect some increase on this or @ojacques' repository, depending on what we recommend using on our docs, so the moment this natively is supported by Material for MkDocs, there's also likely to be more exposure for your plugins, more users, and with that potentially more bugs and edge cases. The question is whether you're up for maintaining it. However, consolidation of efforts would be something to strive for. |
I can certainly help when I am able. Happy to make you maintainer on this repository if that would give you more access, control and autonomy. This is embarrassing, but for some reason I don't always get notified via email of new issues or PRs. Never figured out why. I will look into that so that I can be more aware of issues that come in. |
@byrnereese you can customize notification settings per repository here: |
@ojacques @byrnereese insiders-4.19.0 adds native support for the |
This is so cool! Getting contributors listed in pages has been a big incentive for readers to be turned into contributors. I will look at the first issue reported and continue to submit changes back to upstream. @byrnereese - once / if you agree, happy to be made a maintainer of this repo and continue working with you. |
Awesome work @squidfunk and @ojacques , really cool stuff! |
@ojacques I have not been a good partner in maintaining this project and would like to finally resolve the fact that we have outstanding features in a fork. I have made you a collaborator on this project, and can give you rights to the pypi project as well. Let me know how best to proceed - as I would love to fold your many improvements into the plugin. |
Thanks @byrnereese! Although fulfilling the same goal, the plugin has now a very different implementation, directly getting contributors from GitHub (although not through an API). Along the way, I am not sure I did not broke other usage of this plugin (for example, it will not work with anything other than GitHub). |
@ojacques I think that is a fine direction to be honest. My plugin similarly was tied to github - although it also leveraged plain git. But the linking to people's profiles was very much github-centric. Would it make sense to do a hard merge? I think the risk of breaking people is minor as the utility of the plugin is specifically for mkdocs. But I think it would be good to canonize one of these plugins as the official one. |
While we're discussing the set of git-based mkdocs plugins, it might also be relevant to discuss 'plain git'. I feel there's potential to build a standard, lower-level I've started an implementation at https://github.com/timvink/mkdocs-git-info-plugin that has some basic working code, but it definitely needs a lot more time and careful design before I can integrate into my existing plugins. |
@timvink I am happy to lend a hand in getting the plugin fleshed out more. What specifically will this plugin do and make available to mkdocs developers? Taking a step back, perhaps what all of us on this thread can do is come up with a logic structure for these plugins so that they are designed to work in a standalone manner or in concert with one another. |
First of all, great plugin! I'm evaluating adding native support for this plugin to Material for MkDocs. The main challenge is the increase in build times – they are pretty significant. For instance, the build time for Material for MkDocs went up from 10s to 77s. Would it be possible to cache results?
The plugin would only need to cache the list of contributors per file + the information obtained for each contributor. If this data is normalized, fetching contributor information should only be necessary when there's a new contributor that hasn't been seen before. Other than that, the data is pretty much static – login, name, avatar URL. The name could change, but I'd suspect this is rarely the case. Also, for a page, the data would only need to be re-fetched when the commit SHA changed.
Happy to help.
The text was updated successfully, but these errors were encountered: