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

refactor: Remove WithParams serialization helper, use SER_PARAMS_OPFUNC #28503

Merged
merged 3 commits into from
Oct 31, 2023

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Sep 19, 2023

Every serialization parameter struct already has the SER_PARAMS_OPFUNC, except for one in the tests.

For consistency, and to remove verbose code, convert the test to SER_PARAMS_OPFUNC, and use it everywhere, then remove the WithParams helper.

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 19, 2023

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK ajtowns, TheCharlatan
Concept ACK theuni

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #28451 (Remove unused SER_DISK, SER_NETWORK, SER_GETHASH by maflcko)

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
Copy link
Member

fanquake commented Oct 2, 2023

cc @theuni

@maflcko maflcko force-pushed the 2309-nuke-with-params-force-opfunc- branch from fac9afe to fa544f8 Compare October 3, 2023 09:56
@theuni
Copy link
Member

theuni commented Oct 4, 2023

Concept ACK.

MarcoFalke added 2 commits October 13, 2023 10:30
-BEGIN VERIFY SCRIPT-
 sed -i 's|WithParams(\([a-zA-Z:._]\+\), |\1(|g' $( git grep -l WithParams )
-END VERIFY SCRIPT-
Copy link
Contributor

@TheCharlatan TheCharlatan left a comment

Choose a reason for hiding this comment

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

ACK fa664dc

Copy link
Contributor

@ajtowns ajtowns left a comment

Choose a reason for hiding this comment

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

ACK fa664dc

src/net_processing.cpp Outdated Show resolved Hide resolved
@ajtowns
Copy link
Contributor

ajtowns commented Oct 31, 2023

reACK 9999019

@DrahtBot DrahtBot requested review from TheCharlatan and removed request for ajtowns October 31, 2023 04:25
Copy link
Contributor

@TheCharlatan TheCharlatan left a comment

Choose a reason for hiding this comment

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

Re-ACK 9999019

@fanquake fanquake merged commit d51fb9c into bitcoin:master Oct 31, 2023
16 checks passed
@maflcko maflcko deleted the 2309-nuke-with-params-force-opfunc- branch November 1, 2023 07:13
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.

None yet

6 participants