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

[Diagnostics-Qol]: Update c_style_for_stmt_removed error string #73314

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

saehejkang
Copy link
Contributor

Changes Made

  • Update the c-style-for-stmt-removed error string
  • Updated appropriate tests in the /test directory.

Resolves #73246

@saehejkang
Copy link
Contributor Author

Just had some clarifications when running tests

  1. When do you need to rebuild dependencies when running tests?
  2. This was just updating an error string so does that warrant a rebuild?
  3. What steps should be taken with changes like this (what tests to run)?

@AnthonyLatsis
Copy link
Collaborator

When do you need to rebuild dependencies when running tests?

To put is simply, usually, the products you build to compile, debug and verify your changes are the exact same and only products you want to build before running associated tests. If build times are not a problem, run-test will build test dependencies for you. Otherwise, you learn them along the way. I prefer using lit or skipping the build phase in run-test (--build skip) because run-test is poorly optimized for small subsets of tests — it wastes time rebuilding unrelated stuff.

What steps should be taken with changes like this (what tests to run)?

What I do is search for occurrences of the old diagnostic message in the test suite, update them, and make sure those tests pass.

@AnthonyLatsis
Copy link
Collaborator

@swift-ci please smoke test Linux

@AnthonyLatsis
Copy link
Collaborator

Would be nice to have a tag in the commit title too.

@AnthonyLatsis AnthonyLatsis self-requested a review April 27, 2024 23:43
@saehejkang saehejkang force-pushed the update-c-style-for-stmt-removed-string branch from f6f74fa to 2835376 Compare April 28, 2024 05:36
@LucianoPAlmeida
Copy link
Collaborator

@swift-ci Please smoke test

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.

Change "has been removed" to "was removed".
3 participants