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

NonComparableStreamSort validates direct types #1070

Merged
merged 2 commits into from
Dec 10, 2019

Conversation

carterkozak
Copy link
Contributor

Before this PR

Previously NonComparableStreamSort only failed on types which
would cause a compilation error casting to Comparable. Now we
validate that the stream type is a subtype of Comparable.

After this PR

==COMMIT_MSG==
Improve NonComparableStreamSort to validate that stream types implement comparable, as opposed to validating that casting to comparable does not cause a compiler error.

This commit reduces the severity to WARNING because it's
possible that the check will flag code that happens to work
today, but we strongly recommend against sorting streams of
a type that is not directly comparable without a custom
comparator because it is likely to break later due to lack
of enforcement by the type system.
==COMMIT_MSG==

Possible downsides?

Possible this will cause friction if it flags new things. That friction is substantially lower than failures at runtime.

Previously NonComparableStreamSort only failed on types which
would cause a compilation error casting to Comparable. Now we
validate that the stream type is a subtype of Comparable.

This commit reduces the severity to WARNING because it's
possible that the check will flag code that happens to work
today, but we strongly recommend against sorting streams of
a type that is not directly comparable without a custom
comparator becuase it is likely to break later due to lack
of enforcement by the type system.
@changelog-app
Copy link

changelog-app bot commented Nov 26, 2019

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Improve NonComparableStreamSort to validate that stream types implement comparable, as opposed to validating that casting to comparable does not cause a compiler error.

This commit reduces the severity to WARNING because it's
possible that the check will flag code that happens to work
today, but we strongly recommend against sorting streams of
a type that is not directly comparable without a custom
comparator because it is likely to break later due to lack
of enforcement by the type system.

Check the box to generate changelog(s)

  • Generate changelog entry

@stale
Copy link

stale bot commented Dec 10, 2019

This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days.

@stale stale bot added the stale label Dec 10, 2019
@carterkozak carterkozak removed the stale label Dec 10, 2019
@carterkozak
Copy link
Contributor Author

not stale, just awaiting review

@bulldozer-bot bulldozer-bot bot merged commit b35adda into develop Dec 10, 2019
@bulldozer-bot bulldozer-bot bot deleted the ckozak/NonComparableStreamSort branch December 10, 2019 20:00
This was referenced Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants