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

doc: fix guides according to vale linter recommendations #194

Merged
merged 18 commits into from
Feb 27, 2024
Merged

Conversation

juliamrch
Copy link
Collaborator

@juliamrch juliamrch commented Feb 15, 2024

Checklist

Pull request type

Please check the type of change your PR introduces:

  • Documentation content changes
  • Bugfix on the site
  • Build related changes: start reindexing partials to get a proper TOC
  • Other (please describe):

Description

A summary of the changes

Laravel and Moddle guides have been rewritten for more clarity, partials used in them have been reindexed to get a proper TOC, and even removed from the page when they weren't useful or specific enough.

Changes

This PR contains a slight rework on the /guides section. These are the pages with editorial errors that will be fixed:

Partials that were used (especially in tutorial-laravael.md have been reindexed has specified in this method by @cnivolle. However, I chose not to include them in the Laravel guide. The reasons are detailed in [commit 63d9c25] (63d9c25).

Since reindexing partials could bring conflicts between this branch and main, I've rebased several times to avoid it, therefore this PR contains commits merged to main while working on this branch. Not sure this was smart for readability. When merging, commits should squashed with the content of the last commit.

Further work

This PR only targets rewriting the two aforementioned guides, not the partials. Work on all partials should be completed on a dedicated branch.

Why is this needed?

This is needed in order to improve readability of the guides, but also to progressively set up our own style to improve our editorial check up process in PR, as @davlgd evoked in this PR : #184 (comment)

Moodle guide was flagged by Vale, and I've followed the Laravel one last week: it was crowed with unstructured information and painful to navigate, in my opinion. New version should be clearer and easier to follow. Feel free to add suggestions or to point out errors.

Tests

Both Laravel and Moodle guides have been tested with demo apps:

Reviewes

Who should review these changes? @cnivolle @sebartyr

Copy link

Deployment has finished 👁️👄👁️ Your app is available here

@juliamrch juliamrch self-assigned this Feb 15, 2024
@juliamrch juliamrch added the documentation Improvements or additions to documentation label Feb 15, 2024
Copy link

🚀 Your app has been updated and is available here

@juliamrch juliamrch changed the title doc: fix Moodle guide according to vale linter recommendations doc: fix guides according to vale linter recommendations Feb 15, 2024
cnivolle
cnivolle previously approved these changes Feb 21, 2024
Copy link

🚀 Your app has been updated and is available here

davlgd and others added 7 commits February 21, 2024 14:32
Former template was too detailed and not suited for quick changes like the changelog.

Furthermore, already pre-filled templates seem to reduce author's verbosity. Since there isn't any way of enforcing the filling of a template on PR's like on issues on GitHub, if any additional information is needed it should be requested by the reviewer.
Copy link

🚀 Your app has been updated and is available here

Copy link

🚀 Your app has been updated and is available here

Copy link

🚀 Your app has been updated and is available here

- Remove partials: partials are not specific enough for this guide and their style is too vague for specific frameworks.
- Relocate partials: rewriting implied to relocate partials to get proper TOCs in pages, as sepcified in iss98.
- Set guide in steps: use the steps shortcode to clarify the configuration and deployment process

Discussion: Rewriting partials or deprecating their usage for the most part.
Copy link

🚀 Your app has been updated and is available here

@juliamrch juliamrch added the enhancement New feature or request label Feb 22, 2024
@juliamrch juliamrch marked this pull request as ready for review February 22, 2024 10:14
@juliamrch juliamrch mentioned this pull request Feb 22, 2024
6 tasks
Copy link

🚀 Your app has been updated and is available here

@juliamrch
Copy link
Collaborator Author

@cnivolle would you still merge my PR if I was a worm 🥹?

cnivolle
cnivolle previously approved these changes Feb 26, 2024
Copy link
Member

@cnivolle cnivolle left a comment

Choose a reason for hiding this comment

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

Nice job 👌🏻

Copy link

🚀 Your app has been updated and is available here

@juliamrch
Copy link
Collaborator Author

There is some problem with shortcodes that I do't understand yet. Looking into it : http://doc-review-pr-194.cleverapps.io/doc/applications/dotnet/

add space before title to try to fix the content rendering
Copy link

🚀 Your app has been updated and is available here

@juliamrch
Copy link
Collaborator Author

There is some problem with shortcodes that I do't understand yet. Looking into it : http://doc-review-pr-194.cleverapps.io/doc/applications/dotnet/

solved with a stupid edit (adding a space a top of the partial file)

@cnivolle cnivolle merged commit e333030 into main Feb 27, 2024
5 checks passed
Copy link

Your review app has been deleted 👋

@juliamrch juliamrch deleted the linting-fix branch February 27, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants