Skip to content

docs: Fix toolchain implementation file name#2512

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

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

Conversation

@nicholasjng
Copy link
Contributor

@nicholasjng nicholasjng commented Dec 18, 2024 •
edited by rickeylev
Loading

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.

@nicholasjng
Fix toolchain implementation file name
b0b04f5 
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.
@rickeylev
Copy link
Collaborator

rickeylev commented Dec 18, 2024

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 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
@nicholasjng nicholasjng mentioned this pull request Jan 14, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jan 14, 2025
@nicholasjng
docs: Add horizontal spacers around filenames in custom toolchain gui…
c0bd668 
…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 nicholasjng deleted the fix-impl-filename branch January 15, 2025 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rickeylev rickeylev rickeylev approved these changes

@aignas aignas Awaiting requested review from aignas aignas is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nicholasjng @rickeylev