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

Update make docs procedure #10539

Merged
merged 7 commits into from
Feb 19, 2025
Merged

Update make docs procedure #10539

merged 7 commits into from
Feb 19, 2025

Conversation

github-actions[bot]
Copy link
Contributor

To test the changes, run the following and browse to URL output by the script:

git fetch
git checkout origin/update-make-docs
cd docs
make docs

@github-actions github-actions bot added the type/docs Improvements or additions to documentation label Jan 30, 2025
@github-actions github-actions bot requested review from a team as code owners January 30, 2025 07:02
@jdbaldry jdbaldry requested a review from tacole02 February 7, 2025 12:20
docs/docs.mk Outdated
@@ -120,3 +120,10 @@ update: ## Fetch the latest version of this Makefile and the `make-docs` script
curl -s -LO https://raw.githubusercontent.com/grafana/writers-toolkit/main/docs/docs.mk
curl -s -LO https://raw.githubusercontent.com/grafana/writers-toolkit/main/docs/make-docs
chmod +x make-docs

.PHONY: topic/%
Copy link
Contributor

@narqo narqo Feb 14, 2025

Choose a reason for hiding this comment

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

(nit) Does make resolve % in the phony declarations? I thought the declaration is static so target/% doesn't make target/foo phony, no?

Copy link
Member

Choose a reason for hiding this comment

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

No you're right, that's actually not gonna work if there's a topic/task file. I'll open a PR upstream to fix that behavior and you can expect an update to this PR in the next couple days

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in 8d9899f (#10539)

Copy link
Contributor

@narqo narqo left a comment

Choose a reason for hiding this comment

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

🤖 I've realised I'm commenting to a soulless robot, who doesn't care.

My comment was a nitpick/question, that doesn't make much of a difference. LGTM

@narqo narqo enabled auto-merge (squash) February 14, 2025 10:00
Copy link
Contributor

@tacole02 tacole02 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, @jdbaldry !

@jdbaldry
Copy link
Member

@narqo I don't have write permissions so feel free to merge this when you're happy :)

@narqo narqo merged commit 99118d9 into main Feb 19, 2025
28 checks passed
@narqo narqo deleted the update-make-docs branch February 19, 2025 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants