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.

Sign up for GitHub

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

Jump to bottom

Add a note: prefix to all notes. #4330

Merged
merged 6 commits into from
Sep 24, 2024
Merged

Add a note: prefix to all notes. #4330

merged 6 commits into from
Sep 24, 2024

Conversation

zygoloid
Copy link
Contributor

Omit the note: prefix and the snippet from the "in import" note that precedes a diagnostic.

This makes our diagnostic output more closely match that of Clang and GCC.

@zygoloid
Add a note: prefix to all notes.
c37597d 
Omit the `note: ` prefix and the snippet from the "in import" note that
precedes a diagnostic.

This makes our diagnostic output more closely match that of Clang and GCC.
@github-actions github-actions bot requested a review from chandlerc September 20, 2024 00:08
chandlerc
chandlerc approved these changes Sep 20, 2024
View reviewed changes
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

LGTM as I think this is a good initial step, but see inline -- I think it exposes some additional work we should probably make a note of on the import diagnostics. With the snippets, it wasn't as obviously needed, but seems more important now.

@zygoloid zygoloid requested a review from jonmeow September 20, 2024 23:45
@chandlerc
Copy link
Contributor

(All of this still LG, but good to wait for Jon before landing)

@zygoloid zygoloid enabled auto-merge September 23, 2024 22:17
@zygoloid zygoloid added this pull request to the merge queue Sep 23, 2024
Merged via the queue into carbon-language:trunk with commit 8d45530 Sep 24, 2024
8 checks passed
@zygoloid zygoloid deleted the toolchain-note branch September 24, 2024 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chandlerc chandlerc chandlerc approved these changes

@jonmeow jonmeow jonmeow approved these changes

Assignees
No one assigned
Labels
toolchain
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zygoloid @chandlerc @jonmeow