FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

docs: Fix toolchain implementation file name by nicholasjng · Pull Request #2512 · bazel-contrib/rules_python · GitHub

docs: Fix toolchain implementation file name - #2512

Merged
rickeylev merged 1 commit into
bazel-contrib:mainfrom
nicholasjng:fix-impl-filename
Dec 18, 2024
Merged

docs: Fix toolchain implementation file name#2512
rickeylev merged 1 commit into
bazel-contrib:mainfrom
nicholasjng:fix-impl-filename

Conversation

nicholasjng commented Dec 18, 2024
edited by rickeylev
Loading

Copy link
Copy Markdown
Contributor

The code example comment uses toolchain_impls (extra s), while the target references
use toolchain_impl (no s). This made me go back and double-check when reading the
custom toolchain example (especially the definitions in L425ff).

Remove the extra "s" in the comment so it matches what the target references are.

It's pedantic, but it made me go back and double-check when reading the
custom toolchain example, so I think it's worth preventing this confusion.

Copy link
Copy Markdown
Collaborator

Thanks! Nope, not pedantic at all -- it's confusing if the file names aren't consistent.

re: visual bars: I'd be +1 on that. There's a lot of content in the example, so easy to get lost in what file something is for.

rickeylev added this pull request to the merge queue Dec 18, 2024
Merged via the queue into bazel-contrib:main with commit 9306393 Dec 18, 2024
github-merge-queue Bot pushed a commit that referenced this pull request Jan 14, 2025
…de (#2563)

This improves readability by clearly marking the beginnings and ends of
the three involved source files.

Follow-up of #2512, as discussed.
nicholasjng deleted the fix-impl-filename branch January 15, 2025 10:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL