Skip to content

Avoid creating constructors where not warranted #23178

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

Merged
merged 2 commits into from
May 19, 2025

Conversation

som-snytt
Copy link
Contributor

Avoid creating constructors where not warranted.

Fixes #15144

@som-snytt som-snytt marked this pull request as ready for review May 16, 2025 22:11
@Gedochao Gedochao requested a review from sjrd May 19, 2025 06:38
@sjrd
Copy link
Member

sjrd commented May 19, 2025

Looks good, but the "pre-tweaking" commit does not seem to have anything to do with the real commit of this PR. It should go in a separate refactoring-only PR.

@som-snytt
Copy link
Contributor Author

som-snytt commented May 19, 2025

That is literally why it's in a separate commit. It's in this PR because I had to clean up while wandering around the site, i.e., so-called "scouting".

Maybe there is a better term than "scouting" or "pre-tweaking", such as "twerking", since all twerking is pre-twerking by definition.

Edit: I don't know what to call the post-twerk at scala/scala#11059

Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

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

OK, sure.

FTR I disagree. IMO a separate commit is the least we can do of course, but I would only bundle it in a PR if the changes of that commit support the real commit afterwards (typically because there are colliding affected lines), but not if it's totally unrelated.

Approving not to delay this for no strong reason.

@sjrd sjrd merged commit 51d0d76 into scala:main May 19, 2025
29 checks passed
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.

-release 8 causes java.time.Instant to have a fictitious constructor
2 participants