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] Add spell check instructions #4264

Merged
merged 4 commits into from
Oct 6, 2022

Conversation

mattstein
Copy link
Sponsor Collaborator

@mattstein mattstein commented Oct 5, 2022

The Problem/Issue/Bug:

The enthusiastic writer may not be aware of the spell checker, which would be ideal for the Working on the Docs page.

How this PR Solves The Problem:

This adds a “Check for Spelling Errors” section to that page:

Screen Shot 2022-10-05 at 03 48 08 PM@2x

Terminal commands are grabbed directly from the Makefile, and the is used to indicate user CLI input since that style is used elsewhere in the docs.

Manual Testing Instructions:

https://ddev.readthedocs.io/en/stable/developers/testing-docs/#preview-your-changes

Automated Testing Overview:

Only Markdown; no new tests needed.

Release/Deployment notes:

n/a

@mattstein mattstein self-assigned this Oct 5, 2022
@mattstein mattstein requested a review from rfay October 5, 2022 22:51
@mattstein mattstein marked this pull request as ready for review October 5, 2022 22:54
Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Looks good to me. I don't think the installation works for all environments, but it's a good start.

Want to get them to install markdownlint too, for the same reason?

@mattstein
Copy link
Sponsor Collaborator Author

mattstein commented Oct 5, 2022

Looks good to me. I don't think the installation works for all environments, but it's a good start.

I forget what I did to get it working on my Mac, but I figured it out once I found that Makefile comment. (Trying on another machine now.) Some clues are better than nothing!

Want to get them to install markdownlint too, for the same reason?

Aren’t we already doing that in the section above this newly-added one? If you mean something else, I’m not following.

@rfay
Copy link
Member

rfay commented Oct 5, 2022

You're right, sorry. Markdownlint instructions are already there.

@rfay
Copy link
Member

rfay commented Oct 5, 2022

Is it worth saying that it's OK to just toss it to the build check if they don't want to mess with it?

@mattstein
Copy link
Sponsor Collaborator Author

mattstein commented Oct 5, 2022

I think so! Worth pointing out it’ll be done via CI so nobody’s discouraged from submitting a PR. I shall whip up a blurb—one moment.

Reiterate that nobody *has* to work locally.
@mattstein
Copy link
Sponsor Collaborator Author

mattstein commented Oct 5, 2022

Added this to the top of the page, before someone might get neck-deep into a local checkout:

Screen Shot 2022-10-05 at 04 05 52 PM@2x

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Oh dear. We seem to have lost the whole section I did recently showing this. #4237 - please put that back in, edit as you see fit.

Previously added in 413f7c0.
@mattstein
Copy link
Sponsor Collaborator Author

My fault, sorry! Just re-added.

@mattstein mattstein requested a review from rfay October 5, 2022 23:33
@@ -3,7 +3,16 @@
This section is for people who want to contribute to the DDEV documentation.

!!!tip "No need to work locally!"
The documentation is built and checked automatically with various [GitHub Actions workflows](https://github.com/drud/ddev/actions). It may help to check your work locally, particularly for more involved PRs, but you can just as well make suggestions using GitHub in a browser.
The documentation is built and checked automatically with various [GitHub Actions workflows](https://github.com/drud/ddev/actions). It may help to check your work locally, particularly for more involved PRs, but you can just as well make suggestions using [GitHub in a browser](#fix-docs-using-web-browser).
Copy link
Member

Choose a reason for hiding this comment

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

Do you want this tip to be in the section below about Fork/Clone?

Copy link
Sponsor Collaborator Author

Choose a reason for hiding this comment

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

Good point about the flow. I think it’d be better to remove the tip treatment since it’s less urgent, and use that blurb as context. Commit coming in a moment.

Copy link
Sponsor Collaborator Author

Choose a reason for hiding this comment

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

How’s this feel instead?

@rfay
Copy link
Member

rfay commented Oct 5, 2022

I'm not sure that was your fault that got removed, must have been a merge things, but things like that are always scary. That was definitely something that went in while you were working on this.

@mattstein
Copy link
Sponsor Collaborator Author

I'm not sure that was your fault that got removed, must have been a merge things, but things like that are always scary. That was definitely something that went in while you were working on this.

I just assume I botched the merge commit even though I recall seeing that new section.

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Thanks!

@rfay rfay merged commit 85c86d8 into ddev:master Oct 6, 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.

None yet

2 participants