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

[COPS-1524] Fix plugin logic and config schema #4

Merged
merged 10 commits into from
Aug 15, 2019

Conversation

zhimsel
Copy link
Contributor

@zhimsel zhimsel commented Aug 15, 2019

https://datarobot.atlassian.net/browse/COPS-1524

Changes

Please see this branch's commit history for a list of changes and their explanations

Caveats

This contains breaking changes, and thus any users upgrading to this version will need to update their config.

Tested

I confirmed via manual testing that the new logic and config schema works as expected™️.
I'm defining "as expected" here as "in line with internal MkDocs behavior" and "in accordance with its documentation".

I confirmed that redirects work as expected with all combinations of the following settings:

  • All types of possible redirects (with use_directory_urls set to both true and false):
    • index.md to any
    • README.md to any
    • arbitrary.md to any
    • arbitrary.md to http://google.com
    • redirecting to a non-existent markdown doc produces a warning

This is required because any configuration that doesn't match the
default config schema
(https://github.com/mkdocs/mkdocs/blob/1.0.4/mkdocs/config/defaults.py)
will result in a warning being produced by mkdocs (which fails hard if
strict mode is set).

As per the mkdocs documentation, plugins should specify their own
settings in as sub-objects in their `plugins:` section.

While _technically_ the plugin does read the root-level `redirects:`
config section, it should be using it at all.

Note: this is technically a backwards-incompatible change. Any user who
upgrades their installed plugin to this version will need to modify
their config accordingly.

As such, we should probably make a note of it in the release/version
tag.
Prior to this commit, the logic being used here does not match the logic
that MkDocs uses internally to generate the final site_dir.
More specifically: we don't take into account the `use_directory_urls`
setting at all, which alters how the site_dir is built (in regard to
URLs and paths).
I'm bumping to version 1.0 because this release will contain
backwards-incompatible changes.
@engprod-2
Copy link

engprod-2 bot commented Aug 15, 2019

No DRCODEOWNERS or CODEOWNERS file is found for this repository at master branch.
cc @datarobot/code-and-architecture

Copy link
Contributor

@brian-rickman brian-rickman left a comment

Choose a reason for hiding this comment

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

Looks like a great improvement. Thank you.

README.md Outdated Show resolved Hide resolved
@zhimsel zhimsel merged commit 03dd5fd into master Aug 15, 2019
Copy link
Collaborator

@burkestar burkestar left a comment

Choose a reason for hiding this comment

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

@zhimsel awesome! Thanks for these nice improvements.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@zhimsel zhimsel deleted the zhimsel/COPS-1524/config_and_behavior_fixes branch August 15, 2019 20:44
@zhimsel
Copy link
Contributor Author

zhimsel commented Aug 15, 2019

Would one of you be able to push the new version to PyPi? Or grant me access (zach@himsel.net) to do it myself?

@brian-rickman , @burkestar

@brian-rickman
Copy link
Contributor

I don't have the ability to grant you access on PyPi, but I pushed the new version.
https://pypi.org/project/mkdocs-redirects/1.0.0/

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