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

Social plugin maintenance for MkDocs 1.5.0 #5759

Merged
merged 1 commit into from Jul 27, 2023

Conversation

kamilkrzyskow
Copy link
Collaborator

Hi 👋 ,
after MkDocs 1.5.0 releases and you upgrade the dependency, the config.user_configs property will be marked as deprecated and will trigger a warning message.
Just a pre-emptive maintenance commit ✌️

@kamilkrzyskow
Copy link
Collaborator Author

I hope I wasn't too much pre-emptive... 🙋 let me know if I should draft this PR to avoid a premature merge 😵

@squidfunk
Copy link
Owner

Thanks for the PR! I assume you tested that the plugin still works? We definitely need to add tests at some point, but currently, it's necessary to test changes manually.

@kamilkrzyskow
Copy link
Collaborator Author

I did check it with 1.5.0 (the newest commit of MkDocs).
The PR is supposed to be merged after you change the dependency to MkDocs 1.5.0, merging it now would lead to an error.
That's why I second guessed myself in the other message, as I might've jumped the gun.
To make it work for both now and 1.5.0 would require comparing MkDocs module versions (as the API changes), and since you make releases on top of MkDocs you don't have to do that, as you always know what version is shipped in the dependencies.
I shall just draft it for now to avoid confusion. If I miss the 1.5.0 and don't undraft it before you change the dependencies you can always replicate the changes/merge the commit via git tools other than GitHub ✌️

@kamilkrzyskow kamilkrzyskow marked this pull request as draft July 25, 2023 21:03
@squidfunk
Copy link
Owner

Perfect, thanks for the explanation and thanks for your work! We'll merge it as soon as MkDocs 1.5.0 is released. I understand that without merging it, everything will keep working as expected, except there are now deprecation warnings printed to stdout. That's absolutely fine.

@kamilkrzyskow
Copy link
Collaborator Author

kamilkrzyskow commented Jul 26, 2023

To make it work for both now and 1.5.0 would require comparing MkDocs module versions (as the API changes)

Actually, this statement turned out to be incorrect, because I had to implement support for both 1.5.0 and <1.5.0 in the mkdocs-minify-plugin. With the right order of conditionals, it's possible to use the API change as the deciding factor of which part of the code to run.

https://github.com/kamilkrzyskow/mkdocs-minify-plugin/blob/bc7e1d8339318ed823aeed9e1aaa4f83c442eb30/mkdocs_minify_plugin/plugin.py#L163-L175

I still think that there is no need for this kind of conditional in this project as you decide on which version of MkDocs you depend on. I just thought that the other method is worth mentioning ✌️

@squidfunk
Copy link
Owner

MkDocs 1.5.0 is out. We'll be updating to MkDocs >= 1.5.0 as it's out now and release a bugfix release shortly. Material for MkDocs 9.2.x will also depend on MkDocs 1.5.0 and will be considered stable soon.

@kamilkrzyskow kamilkrzyskow marked this pull request as ready for review July 26, 2023 22:02
@kamilkrzyskow
Copy link
Collaborator Author

Great, then I will just open this PR again and let you decide when to merge it 💪

@squidfunk squidfunk merged commit 772421d into squidfunk:master Jul 27, 2023
3 checks passed
@squidfunk
Copy link
Owner

Thanks again! We'll issue 9.1.20 and 9.2.0b2 in a moment.

@kamilkrzyskow
Copy link
Collaborator Author

@squidfunk
I see that the MkDocs dependency is still 1.4.2

mkdocs>=1.4.2

pip typically installs the newest package if there is >=, so in this case 1.5.0, but if a user has MkDocs 1.4.2 or 1.4.3 already installed on their system doing a pip install mkdocs-material would keep the 1.4.2 version, as the dependency list still allows for 1.4.2.
In this case of events users will get errors using the social plugin:

    if theme.custom_dir:
       ^^^^^^^^^^^^^^^^
AttributeError: 'Theme' object has no attribute 'custom_dir'

and since you make releases on top of MkDocs you don't have to do that, as you always know what version is shipped in the dependencies

I said that previously, because I expected you'd update the dependency in the requirements.txt, sorry for not being clear enough 😞

@squidfunk
Copy link
Owner

squidfunk commented Jul 27, 2023

Hmm, okay, so I misunderstood this then. What I did: I merged your PR and tested with MkDocs 1.4.3 (which I had on my system) if the build still works when I add the social plugin – all good. Then, I merged the PR, as I assumed that it's backward compatible. I think it was because I had a fresh cache. We'll bump 9.1 to MkDocs 1.5.0 then and issue a new release.

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

2 participants