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

Juju ecosystem docs post-release fixes #569

Merged
merged 5 commits into from
Feb 7, 2025

Conversation

abbiesims
Copy link
Contributor

@abbiesims abbiesims commented Feb 7, 2025

Done

  • Makes link / textual changes according to this doc

QA

  • Check the demo and make sure links / text from doc has been updated

Issue / Card

Fixes WD-19118

@webteam-app
Copy link

Copy link
Member

@bartaz bartaz left a comment

Choose a reason for hiding this comment

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

LGTM with minor comments

@bartaz
Copy link
Member

bartaz commented Feb 7, 2025

still LGTM in terms of code and QA

@bartaz
Copy link
Member

bartaz commented Feb 7, 2025

I find it a bit odd how different sections start with lowercase "with something"…

image

I find it off in terms of design and when I scan and read it it doesn't read well.

I would find it better as either just "Charmcraft and Flask" or "Build with Charmcraft and Flask". This would also be much better for accessibility and screenreaders that may be reading text out of context of the page and headings above.

@tmihoc
Copy link
Member

tmihoc commented Feb 7, 2025

@bartaz Thank you for your reaction on the Build charmed applications section. However, please note that "with Charmcraft/Flask" etc. here is supposed to mean "with Charmcraft used with the Charmcraft extension that makes charming Flask applications easy", so "Charmcraft and Flask" and "Build with Charmcraft and Flask" would not be correct paraphrases. I cannot think of better short paraphrases right now either. Feels like something that will require some more thought. Can we leave that work for a future PR?

@bartaz
Copy link
Member

bartaz commented Feb 7, 2025

@tmihoc Yes, I don't mean to block this PR on that, but wanted to comment for future reference (as I see the similar approach is used across the page).

@abbiesims abbiesims merged commit 2eee27f into main Feb 7, 2025
6 checks passed
@abbiesims abbiesims deleted the WD-19118/ecosystem-docs-fixes branch February 7, 2025 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants