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

Check docs are updated in prs #68

Merged
merged 5 commits into from
Nov 18, 2022
Merged

Check docs are updated in prs #68

merged 5 commits into from
Nov 18, 2022

Conversation

charlieegan3
Copy link
Contributor

Fixes #66

This adds a check that docs are generated when commands are added and changed!

@irbekrm
Copy link
Contributor

irbekrm commented Nov 18, 2022

Thank you for working on this Charlie.

At the moment these docs are jsctl's only documentation, so I think ./docs should have docs for the latest released version of jsctl, not the latest main. This check would force us to do the latter. I think it would be better if instead we had automation to PR docs updates against this repo as part of release process #66 (comment)

@charlieegan3
Copy link
Contributor Author

Ahh, that's what was meant in #66. I wasn't sure but thought that this made sense anyway.

I personally still think this is the right thing to do for now. It seems odd to me for the docs in the repo to not match the state of the code at that same version. I think that's a desirable property.

The reality is that a user landing on the repo looking for docs is that they could be using any version, rather than the latest release.

I agree that it'd be nice to make it possible to browse different versions of the docs site. However, for now, I still feel this is a step in the right direction.

Copy link
Contributor

@irbekrm irbekrm left a comment

Choose a reason for hiding this comment

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

match the state of the code at that same version

I've come to see your point yeah. We can always link the docs at a specific tag for a specific release.

Thanks for working on this!

/lgtm

@charlieegan3
Copy link
Contributor Author

Thanks Irbe.

@charlieegan3 charlieegan3 merged commit b2dc875 into main Nov 18, 2022
@charlieegan3 charlieegan3 deleted the check-docs branch November 18, 2022 16:37
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.

Run make docs-gen to ensure docs are up-to-date in PRs
3 participants