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

[Docs] Remove examples section and point through to gallery #814

Merged
merged 12 commits into from
Oct 22, 2024

Conversation

stichbury
Copy link
Contributor

@stichbury stichbury commented Oct 17, 2024

Description

Build version of PR is here

Discussed here: #629
(Internal ticket #1205

  • Moved relevant useful text from the examples page to the FAQ section.
  • Updated the front page to point through to the gallery, and updated the information architecture. (It would be good if the menu opened the gallery in a separate tab/window if possible -- I'm not sure how to do this with mkdocs)
  • Moved your-examples.md to the explanation section
  • Added @huong-li-nguyen's Charming Data video to the "Your examples" page

Screenshot

Notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

    • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
    • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
    • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
    • I have not referenced individuals, products or companies in any commits, directly or indirectly.
    • I have not added data or restricted code in any commits, directly or indirectly.

Copy link
Contributor

@huong-li-nguyen huong-li-nguyen left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@stichbury
Copy link
Contributor Author

stichbury commented Oct 17, 2024

@antonymilne so this is an example of where it's flagging that I need a changelog despite only modifying docs. Presumably it's mkdocs.yml that is outside of the docs folder. Is there a way we can add this to the set of exceptions? Very minor importance, I don't have a problem with adding a changelog, but wanted to raise that there are cases where pure docs changes need them.

@huong-li-nguyen
Copy link
Contributor

@antonymilne so this is an example of where it's flagging that I need a changelog despite only modifying docs.

I think it's because you change the mkdocs.yml, which is outside the docs folder. Every change inside the docs folder doesn't require a changelog file, but I agree with you - we should also ignore the mkdocs.yml if possible 🤔

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.

Thank you for doing this! Looks good, but I have a few comments and questions that I'd like @maxschulz-COL to consider before we merge.

For the link opening in a new window vs. current one: I think this is fine how it is, since generally speaking we should prefer to open in the same tab. On the homepage we could replace the markdown link with an HTML one that includes "opens in new page" icon, but I don't think this would look good on the top bar of the navigation. So tbh I'd just leave it how you have it.

As for the changelog being needed: as you and Li said, this is because you modified mkdocs.yml that lives outside the docs. We could put this in so it's treated as part of the docs, but now that I think about it again I actually think we should change the logic so that a changelog is required for only changes to src. So docs and everything else outside src would all be excluded rather than mkdocs.yml being a special exclusion. wdyt @maxschulz-COL>? My reasoning is:

  1. changelogs are really only required when there's a change to the package that affects users, which necessarily means changing code in src
  2. in case there's some edge cases (e.g. dropping a Python version), you can just manually add a changelog

Overall I think this will be a slightly smoother process since a changelog would only be demanded when it might be needed.

vizro-core/docs/pages/explanation/your-examples.md Outdated Show resolved Hide resolved
vizro-core/docs/pages/explanation/faq.md Outdated Show resolved Hide resolved
@maxschulz-COL
Copy link
Contributor

So docs and everything else outside src would all be excluded rather than mkdocs.yml being a special exclusion. wdyt @maxschulz-COL>?

I think that is fine, let's go for that

Copy link
Contributor

@maxschulz-COL maxschulz-COL left a comment

Choose a reason for hiding this comment

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

Lgtm! I like it more now, highlights our examples page more, and hyou can still get to user examples quickly

vizro-core/docs/pages/explanation/your-examples.md Outdated Show resolved Hide resolved
@stichbury stichbury enabled auto-merge (squash) October 22, 2024 08:40
@stichbury stichbury merged commit a2ba36e into main Oct 22, 2024
31 checks passed
@stichbury stichbury deleted the docs/remove-examples-section branch October 22, 2024 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs 🗒️ Issue for markdown and API documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants