Skip to content

tidy: add modernize-use-default-member-init #25108

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 2 commits into from
May 18, 2022
Merged

tidy: add modernize-use-default-member-init #25108

maflcko merged 2 commits into from
May 18, 2022

Conversation

@fanquake
Copy link
Member

@fanquake fanquake commented May 11, 2022 •
edited
Loading

Refactor and then enable modernize-use-default-member-init in our clang-tidy job.

PastaPastaPasta
PastaPastaPasta reviewed May 11, 2022
View reviewed changes
Copy link
Contributor

@PastaPastaPasta PastaPastaPasta left a comment •
edited
Loading

Choose a reason for hiding this comment

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

while you are here, I think it'd make sense to use = default

Otherwise, concept ACK

@DrahtBot DrahtBot mentioned this pull request May 12, 2022
Closed
@DrahtBot
Copy link
Contributor

DrahtBot commented May 12, 2022 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #25110 (tidy: use modernize-use-raw-string-literal by fanquake)
  • #24914 (wallet: Load database records in a particular order by achow101)
  • #24676 ([WIP] [kernelheaders 1/n] Cleave LevelDB headers from our header tree by dongcarl)
  • #24149 (Signing support for Miniscript Descriptors by darosior)
  • #22793 (Simplify BaseSignatureChecker virtual functions and GenericTransactionSignatureChecker constructors by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake fanquake force-pushed the modernize_use_default_Member_init branch 2 times, most recently from 26ad839 to faeb05e Compare May 13, 2022 11:32
@fanquake
Copy link
Member Author

fanquake commented May 13, 2022

while you are here, I think it'd make sense to use = default

Have done that now.

PastaPastaPasta
PastaPastaPasta approved these changes May 13, 2022
View reviewed changes
Copy link
Contributor

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK, I have reviewed the diff, and all changes make sense. I would recommend changing the title to refactor: clang-tidy enable/use modernize-use-default-member-init and modernize-use-equals-default

@fanquake fanquake force-pushed the modernize_use_default_Member_init branch from faeb05e to 0041baf Compare May 13, 2022 15:48
This was referenced May 14, 2022
Closed
Merged
Merged
@fanquake fanquake force-pushed the modernize_use_default_Member_init branch from 0041baf to ac6fbf2 Compare May 17, 2022 16:29
@DrahtBot DrahtBot mentioned this pull request May 17, 2022
Closed
@maflcko
Copy link
Member

maflcko commented May 18, 2022

ACK modernize-use-default-member-init

not sure if modernize-use-equals-default provides any value for us, but I guess it doesn't hurt either.

@maflcko maflcko merged commit 7b3343f into bitcoin:master May 18, 2022
@fanquake fanquake deleted the modernize_use_default_Member_init branch May 19, 2022 08:41
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 28, 2022
Merge bitcoin#25108: tidy: add modernize-use-default-member-init
e8b1066
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 18, 2022
@PastaPastaPasta
Merge bitcoin#25108: tidy: add modernize-use-default-member-init
dda7a65 
ac6fbf2 tidy: use modernize-use-default-member-init (fanquake)
7aa40f5 refactor: use C++11 default initializers (fanquake)

Pull request description:

  Refactor and then enable [`modernize-use-default-member-init`](https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-default-member-init.html) in our `clang-tidy` job.

Top commit has no ACKs.

Tree-SHA512: 536b406f20639f8c588fe9e96175ec60c7bb825506b2670b562370b2f572801c24203c483443be3c199e1b958c0765d4532e57c57a4e78689162a1dd422d844f
8498549767
8498549767 reviewed Jan 24, 2023
View reviewed changes
Copy link

@8498549767 8498549767 left a comment

Choose a reason for hiding this comment

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

Exelente

@bitcoin bitcoin locked and limited conversation to collaborators Jan 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@8498549767 8498549767 8498549767 left review comments

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@fanquake @DrahtBot @maflcko @PastaPastaPasta @8498549767