Skip to content

Conversation

@lorentey
Copy link
Member

Change some unfortunate wording around invariant violations.

Checklist

  • I've read the Contribution Guidelines
  • My contributions are licensed under the Swift license.
  • I've followed the coding style of the rest of the project.
  • I've added tests covering all new code paths my change adds to the project (if appropriate).
  • I've added benchmarks covering new functionality (if appropriate).
  • I've verified that my change does not break any existing tests or introduce unexplained benchmark regressions.
  • I've updated the documentation if necessary.

@lorentey
Copy link
Member Author

@swift-ci test

@vanvoorden
Copy link
Contributor

vanvoorden commented Oct 21, 2024

@lorentey I believe soundness would also be a legit option to replace those.

FWIW… there might also be examples of outdated language that can't be deleted easily like comments… there could be constant strings passed to logger or even symbols like function names that contain the outdated language. You might also for this specific diff think about leaving these comments in but just migrating the language to something modern and inclusive as a guide for future engineers to migrate future symbols.

cc @Serena748

@lorentey
Copy link
Member Author

Yep.

@lorentey
Copy link
Member Author

C.f. #426

@lorentey
Copy link
Member Author

I'm not particularly fond of the idea that assertions need to explicitly explain that they're for checking invariants. (That is what assertions are for.) Therefore, the right fix was to remove these.

Please file PRs or issues if you find any similar cases. These were found by an automated tool; I find it encouraging that the only hits were in experimental code that hasn't yet gone through the usual implementation/style review.

@lorentey lorentey merged commit 54e9986 into apple:main Oct 21, 2024
@lorentey lorentey deleted the fix-linter-issues branch October 21, 2024 18:38
@lorentey lorentey added this to the 1.3.0 milestone Sep 27, 2025
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