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

Migrate starter template docs from recommonmark to myst_parser #1503

Merged
merged 8 commits into from
May 16, 2022

Conversation

avan-sh
Copy link
Contributor

@avan-sh avan-sh commented May 4, 2022

Description

PR #1489 addresses the bulk of the changes required for migrating to myst-parser in docs, this pr modifies conf.py in templates which is used by kedro build-docs command in new projects.

Closes #1479

Note: While this is likely to not be breaking for new projects, docs copied directly from older projects might need to be updated.

Development notes

  • Modified docs/conf.py & setup.py to remove references to recommonmark and update it to myst_parser.
  • Updated test for kedro build docs.

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes

avan-sh added 2 commits April 30, 2022 11:51
Signed-off-by: Avaneesh Yembadi <[email protected]>
@avan-sh avan-sh requested review from yetudada and idanov as code owners May 4, 2022 16:12
avan-sh added 2 commits May 5, 2022 18:47
Signed-off-by: Avaneesh Yembadi <[email protected]>
Signed-off-by: Avaneesh Yembadi <[email protected]>
Copy link
Contributor

@antonymilne antonymilne left a comment

Choose a reason for hiding this comment

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

Ah nice! I was just saying to @MerelTheisenQB yesterday that we should make this change but might not get around to it since probably we should reconsider the whole kedro build-docs command, whether to continue using sphinx, update other bits in conf.py because it's very old...

Anyway, this definitely seems like a good improvement to the current setup, thank you! Please can you add a note to RELEASE.md and also make the change to kedro-starters?

@avan-sh
Copy link
Contributor Author

avan-sh commented May 6, 2022

Ah nice! I was just saying to @MerelTheisenQB yesterday that we should make this change but might not get around to it since probably we should reconsider the whole kedro build-docs command, whether to continue using sphinx, update other bits in conf.py because it's very old...

@AntonyMilneQB Is this just about how docs are built within a kedro project? If this is already on cards, this change might not be worth it as introducing two breaking changes in succession might not be worth it.

I could also take a crack at the refactor/clean-up if you could elaborate a bit on what/why we're looking to do.

@antonymilne
Copy link
Contributor

antonymilne commented May 6, 2022

@avan-sh Yeah, this is just about how docs are build within a kedro project. I'd say that future changes are not imminently on the cards, so I think it's still worth making this change now.

Given that this change doesn't actually touch the kedro build-docs command and is just a small change to the template I don't think it's really a breaking change, because kedro build-docs will still work the same way on projects using an old template. Also the subset of users who use kedro build-docs + use markdown documentation + embed RST in their markdown is so small anyway - it's kind of a weird documentation setup that we have on kedro and just got copied to the project template.

Equally you could argue that the number of affected users is so small that it's just not worth the bother of updating it, since no one has noticed that the project template uses an old sphinx extension...

I could also take a crack at the refactor/clean-up if you could elaborate a bit on what/why we're looking to do.

Thank you for the offer but I think it's a bit premature to actually do anything about this because we haven't at all worked out what to do with it yet 😅 I was just mentioning it as a vague future plan - we still need to check telemetry data, do user research, etc. Probably needs to be considered as part of the whole vaguely-planned streamlining of the CLI and #1293.

@deepyaman
Copy link
Member

deepyaman commented May 6, 2022

I'd say that future changes are not imminently on the cards, so I think it's still worth making this change now.

👍

Given that this change doesn't actually touch the kedro build-docs command and is just a small change to the template I don't think it's really a breaking change, because kedro build-docs will still work the same way on projects using an old template.

👍

Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

Thanks for making this change @avan-sh ! ⭐ 👍

@merelcht merelcht merged commit c6882f6 into kedro-org:main May 16, 2022
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.

Migrate from recommonmark to MyST
4 participants