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

src: fix modernize-return-braced-init-list #26023

Closed
wants to merge 1 commit into from

Conversation

gengjiawen
Copy link
Member

@gengjiawen gengjiawen commented Feb 10, 2019

Fix https://clang.llvm.org/extra/clang-tidy/checks/modernize-return-braced-init-list.html.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Feb 10, 2019
@gengjiawen
Copy link
Member Author

@addaleax Can you review this, thanks.

@addaleax
Copy link
Member

@gengjiawen I’m not sure I see the harm in being explicit about return types…

@gengjiawen gengjiawen closed this Feb 12, 2019
@gengjiawen
Copy link
Member Author

Give another thought, I agree with you.

@gengjiawen
Copy link
Member Author

But from another way, this is less verbose in most cases ...
I am not sure which style should we follow.

Any opinion ?
@refack @bnoordhuis @jasnell @joyeecheung @cjihrig @targos

@refack
Copy link
Contributor

refack commented Feb 14, 2019

Personally I prefer being tarse, but the concensus seems to be with being explicit #23028

@gengjiawen gengjiawen reopened this Feb 17, 2019
@gengjiawen
Copy link
Member Author

Reopen it to give this more discussion.

@devsnek
Copy link
Member

devsnek commented Feb 17, 2019

i've literally never seen this before, and i suspect the same is true of others. i feel this would be more confusing than anything else.

@joyeecheung
Copy link
Member

joyeecheung commented Feb 17, 2019

I can't really see the benefit this brings, aside from that it makes the return statements more succinct and potentially keeps them separate from return type changes, but then in the case of Maybes I like to see them explicit so it's easier to tell whether you are propagating the JS errors properly.

@gengjiawen
Copy link
Member Author

Perhaps another time.

@gengjiawen gengjiawen closed this Feb 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants