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: Tidy up docs-related make targets #5167

Merged
merged 4 commits into from
Feb 25, 2022

Conversation

matej-g
Copy link
Collaborator

@matej-g matej-g commented Feb 17, 2022

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Make targets to check and / or generate docs can be confusing since:

  1. We have target called check-docs which however also generates and formats docs
  2. We have docs target, which generates docs but this target is unused at the moment

The suggested way to improve this is:

  1. Make docs target generate and format docs correctly, including removing white noise.
  2. Make check-docs target check generated docs against discrepancies in formatting, links and white noise.

Also fixes CHANGELOG formatting issue that is currently in main.

Verification

Ran the targets locally.

Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
@Nexucis Nexucis mentioned this pull request Feb 17, 2022
2 tasks
@hanjm hanjm mentioned this pull request Feb 18, 2022
2 tasks
saswatamcode
saswatamcode previously approved these changes Feb 19, 2022
Copy link
Member

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

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

This looks good to me! 👍

@@ -28,6 +28,6 @@ jobs:
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}

- name: Check docs
run: make check-docs
run: make docs
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit worried about this name change because it seems inconsistent. I think that check-* is more in line with our other make targets, for example the one below, which are designed to exit non-zero when something is wrong

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right, this does not fit nicely with other targets. I changed this to:

  • Have docs target correctly generate and format the docs, including white noise handling
  • Keep check-docs as a target to check against discrepancies and exiting with error if some are found

Signed-off-by: Matej Gera <matejgera@gmail.com>
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

💪

@GiedriusS GiedriusS merged commit 5e123ae into thanos-io:main Feb 25, 2022
Nicholaswang pushed a commit to Nicholaswang/thanos that referenced this pull request Mar 6, 2022
* Tidy up doc targets

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Fix CHANGELOG formatting

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Adjust Make targets after feedback

Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Nicholaswang <wzhever@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants