Skip to content

creator-applications.md: Improve readability of source code comment #11144

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 1 commit into from
Feb 4, 2021

Conversation

Flowdalic
Copy link
Contributor

One can not easily identify the start of the "code section" in "same as new StringBuilder()". Instead, use a colon to separate words from code.

One can not easily identify the start of the "code section" in "same as new StringBuilder()". Instead, use a colon to separate words from code.
@Flowdalic
Copy link
Contributor Author

CLA signed, please re-check.

@OlivierBlanvillain
Copy link
Contributor

OlivierBlanvillain commented Jan 18, 2021

CLA ✔️

I agree that a colon is missing here, but I think // same as: new ... would be better than // old: .... "Same as" carries extra information (both sides are identical), where as "old" makes it sound like a refactoring...

@Flowdalic
Copy link
Contributor Author

Flowdalic commented Jan 18, 2021

I deliberately decided against // same as: new because it gives the impression that both approaches are equal. However the Motivation for Universal Apply Methods states that leaving out new "makes code more pleasant to read.", and hence should be preferred. This is why I tried to discourage the usage of new by calling it "old".

How about

StringBuilder("abc")  // legacy: new StringBuilder("abc")
StringBuilder()       // legacy: new StringBuilder()

But then again, I do like old, as this describes a old way of doing things.

OTOH, I am aware that this could be considered nitpick-ish and it is definitely subjective. So, what should it be?

@odersky odersky merged commit c00b289 into scala:master Feb 4, 2021
@Flowdalic Flowdalic deleted the patch-1 branch February 4, 2021 18:11
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