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 validation Github Action for docs PRs #59

Merged
merged 23 commits into from
Jul 22, 2024
Merged

docs: Add validation Github Action for docs PRs #59

merged 23 commits into from
Jul 22, 2024

Conversation

grafakus
Copy link
Contributor

@grafakus grafakus commented Jul 16, 2024

✨ Description

Related issue(s): resolves #62

This PR:

  • adds a GitHub action to build the docs when documentation files are modified in a PR,
  • prevents frontend build and tests to be executed when only docs files have been modified in a PR
  • adds an npm script to help contributing to the documentation.
  • formats all Markdown docs with Prettier (the ones that were not formatted yet),

📖 Summary of the changes

See diff tab

🧪 How to test?

  • The build with the new Github Action should pass

# Conflicts:
#	CHANGELOG.md
@grafakus grafakus changed the title docs: Linting docs: Automatic linting Jul 16, 2024
@github-actions github-actions bot added the type/docs Used by Docs team for project management label Jul 16, 2024
Copy link
Contributor

github-actions bot commented Jul 16, 2024

Unit test coverage

Lines Statements Branches Functions
Coverage: 12%
13.05% (468/3584) 10.43% (138/1322) 9.9% (107/1080)

simonswine
simonswine previously approved these changes Jul 16, 2024
Copy link
Contributor

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

LGTM

Not fully sure about the implications for the docs in docs/sources and other validations/builds docs/Makefile`. Are we even building the docs as part of a PR?

But gnerally agree with enforcing this

@knylander-grafana
Copy link
Contributor

knylander-grafana commented Jul 16, 2024

LGTM

Not fully sure about the implications for the docs in docs/sources and other validations/builds docs/Makefile`. Are we even building the docs as part of a PR?

We should build docs as part of the PR check if the PR has doc changes like we do in the Pyroscope repo. Our build process doesn't use any docs outside of docs/sources, so the linter should only run on those items. If you have specific questions, I can ask Jack Baldry and find out for you.

But generally agree with enforcing this

For reference, doc linter info: (https://grafana.com/docs/writers-toolkit/review/lint-prose/).

Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

Nice, most looks good to me

README.md Outdated Show resolved Hide resolved
@jdbaldry jdbaldry mentioned this pull request Jul 16, 2024
@grafakus grafakus changed the title docs: Automatic linting docs: Disable Prettier for the "docs" folder Jul 17, 2024
@grafakus grafakus changed the title docs: Disable Prettier for the "docs" folder docs: Disable Prettier for the "docs" folder, add relevant scripts Jul 17, 2024
.prettierignore Outdated Show resolved Hide resolved
.github/workflows/ci-build-docs.yml Outdated Show resolved Hide resolved
@knylander-grafana
Copy link
Contributor

Since Jack has already reviewed, I'll defer to his comments.

@grafakus grafakus changed the title docs: Disable Prettier for the "docs" folder, add relevant scripts docs: Add validation Jul 18, 2024
@grafakus grafakus changed the title docs: Add validation docs: Add validation Github Action for PRs Jul 18, 2024
@grafakus grafakus changed the title docs: Add validation Github Action for PRs docs: Add validation Github Action for docs PRs Jul 18, 2024
@grafakus grafakus requested a review from jdbaldry July 18, 2024 11:47
jdbaldry
jdbaldry previously approved these changes Jul 19, 2024
Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

Docs related CI and DX look good to me

# Conflicts:
#	docs/sources/_index.md
@grafakus grafakus requested a review from jdbaldry July 19, 2024 14:00
@grafakus
Copy link
Contributor Author

@jdbaldry I've fixed some conflicts, may I ask you a final review? 🙏🏾 Thank you!

Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

LGTM!

@grafakus grafakus merged commit f813a05 into main Jul 22, 2024
3 of 4 checks passed
@grafakus grafakus deleted the doc/linting branch July 22, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/docs Used by Docs team for project management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build docs as part of CI
4 participants