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

[NFC] Fix note about manually generated test output #7221

Merged
merged 2 commits into from
Jan 15, 2025

Conversation

tlively
Copy link
Member

@tlively tlively commented Jan 15, 2025

The note mentioned update_lit_checks.py in the first line, which made
update_lit_checks.py think that it should update the test output, even
though the comment was explicitly about how it should not update the
test output. Reword the comment to avoid the problem.

Also make the check in the update script stricter for good measure.

The note mentioned update_lit_checks.py in the first line, which made
update_lit_checks.py think that it _should_ update the test output, even
though the comment was explicitly about how it should not update the
test output. Reword the comment to avoid the problem.
@tlively tlively requested review from kripken and dschuff January 15, 2025 21:40
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm, but how about making the check look for the full string and not just update_lit_checks.py specifically?

@tlively
Copy link
Member Author

tlively commented Jan 15, 2025

Good idea, done. (At least, to the extent that it wouldn't become more complicated.)

@tlively tlively enabled auto-merge (squash) January 15, 2025 22:12
@tlively tlively merged commit c582f8d into main Jan 15, 2025
13 checks passed
@tlively tlively deleted the fix-manual-test-comment branch January 15, 2025 23:44
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.

3 participants