Skip to content

ci: add validation for generated docs - #4187

Merged
thaJeztah merged 2 commits into
docker:masterfrom
dvdksn:docs/validate-markdown
Apr 12, 2023
Merged

ci: add validation for generated docs#4187
thaJeztah merged 2 commits into
docker:masterfrom
dvdksn:docs/validate-markdown

Conversation

@dvdksn

@dvdksndvdksn commented Apr 11, 2023

Copy link
Copy Markdown
Contributor

Signed-off-by: David Karlsson david.karlsson@docker.com

- What I did

Added a step for validating the generated markdown/yaml files, based on the generation output vs what's checked in to the repo

closes#4171

- How I did it

Added a job to the validate GHA workflow

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@dvdksn

Copy link
Copy Markdown
ContributorAuthor

Well that failed spectacularly, love it

Comment thread.github/workflows/validate.yml Outdated
@dvdksn
dvdksnforce-pushed the docs/validate-markdown branch 3 times, most recently from 9f6962e to 8b76701CompareApril 11, 2023 13:39
@dvdksn

Copy link
Copy Markdown
ContributorAuthor

alright, seems to work. Do you want me to push a failing commit to verify? @thaJeztah

@thaJeztah

Copy link
Copy Markdown
Member

Awesome! Let me also call in @crazy-max to check 🤗

@crazy-maxcrazy-max left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

In follow-up it would be nice to be able to run it locally. Probably sandboxed with a Dockerfile like we do in buildx repo.

Comment thread.github/workflows/validate.yml Outdated
@codecov-commenter

codecov-commenter commented Apr 12, 2023

Copy link
Copy Markdown

Codecov Report

Merging #4187 (5046967) into master (fff5a31) will decrease coverage by 0.18%.
The diff coverage is n/a.

Additional details and impacted files
@@ Coverage Diff @@## master #4187 +/- ##
==========================================
- Coverage 59.03% 58.85% -0.18% 
==========================================
Files 288 572 +284 Lines 24776 49540 +24764 ==========================================
+ Hits 14627 29158 +14531 - Misses 9265 18614 +9349 - Partials 884 1768 +884 

@thaJeztahthaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need the fetch-depth: 0 at line 33, or can that one also be removed?

If we need to keep it for that one, we should probably add a comment to explain "why" it's needed for that one (which may help prevent others copying that when creating new checks).

./cc @crazy-max

Comment thread.github/workflows/validate.yml Outdated
Comment on lines -47 to -48
with:
fetch-depth: 0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

micro-nit: this should probably be in the first commit (so that we don't add it in the first place 😄

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

sorted it out 😆

@crazy-max

Copy link
Copy Markdown
Member

Do we need the fetch-depth: 0 at line 33, or can that one also be removed?

If we need to keep it for that one, we should probably add a comment to explain "why" it's needed for that one (which may help prevent others copying that when creating new checks).

./cc @crazy-max

It looks safe to remove 👍

Signed-off-by: David Karlsson <david.karlsson@docker.com>
Signed-off-by: David Karlsson <david.karlsson@docker.com>
@dvdksn
dvdksnforce-pushed the docs/validate-markdown branch from 5046967 to 341c843CompareApril 12, 2023 11:32

@thaJeztahthaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

Sign up for freeto 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.

CI: validate generated Markdown as part of CI

4 participants

@dvdksn@thaJeztah@codecov-commenter@crazy-max