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

fix: correct link to toolchain documentation #1978

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

mgred
Copy link
Contributor

@mgred mgred commented Dec 10, 2021

PR Checklist

PR Type

What kind of change does this PR introduce?

  • Documentation content changes

What is the current behavior?

The link on How to use the Docker Toolchain in docs/container.md neither works in Github nor in the online documentation

What is the new behavior?

The link now works and points to the right page and anchor.

Does this PR introduce a breaking change?

  • Yes
  • No

@@ -946,7 +946,7 @@ def container_image(**kwargs):
layers inside the Docker registry.

This rule references the `@io_bazel_rules_docker//toolchains/docker:toolchain_type`.
See [How to use the Docker Toolchain](toolchains/docker/readme.md#how-to-use-the-docker-toolchain) for details.
See [How to use the Docker Toolchain](https://github.com/bazelbuild/rules_docker/tree/master/toolchains/docker/readme.md#how-to-use-the-docker-toolchain) for details.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would break previewing at a specific PR. Could we change do a similar change except replace https://github.com/bazelbuild/rules_docker/tree/master/ with /? That will allow GitHub to rewrite the urls for a specific branch/pr/commit. This is very useful for looking into the past for docs.

Copy link
Contributor Author

@mgred mgred Dec 19, 2021

Choose a reason for hiding this comment

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

@gravypod thanks for your review and suggestions! I made the change and tested it, perfectly works. This will still not work for the documentation published under docs.aspect.dev. Is this something we should have in mind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I fixed and rebased already 🙃

@gravypod gravypod merged commit 411267d into bazelbuild:master Dec 20, 2021
@mgred mgred deleted the fix-docs branch December 20, 2021 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants