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

Merge bitcoin#14771: test: Add BOOST_REQUIRE to getters returning optional #4297

Merged
merged 2 commits into from
Jul 29, 2021

Conversation

5tefan
Copy link

@5tefan 5tefan commented Jul 28, 2021

Merges bitcoin#14771: test: Add BOOST_REQUIRE to getters returning optional

fa21ca0 test: Add BOOST_REQUIRE to getters returning optional
(MarcoFalke)

Pull request description:

Usually the returned value is already checked for equality, but for
sanity we might as well require that the getter successfully returned.

…ional

fa21ca0 test: Add BOOST_REQUIRE to getters returning optional
            (MarcoFalke)

Pull request description:

  Usually the returned value is already checked for equality, but for
sanity we might as well require that the getter successfully returned.
@5tefan
Copy link
Author

5tefan commented Jul 28, 2021

There's a second component of bitcoin#14771, which seems to be adding NODISCARD. It looks like Dash is generally not adding NODISCARD nor the [[nodiscard]] attribute.

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

It looks like Dash is generally not adding NODISCARD nor the [[nodiscard]] attribute.

Yeah, it's rare but we do have it in a few places e.g. https://github.com/dashpay/dash/blob/develop/src/evo/deterministicmns.h#L511. I think this part should be backported as well (NOTE: pls use [[nodiscard]] instead of NODISCARD).

@UdjinM6 UdjinM6 added this to the 18 milestone Jul 28, 2021
@5tefan 5tefan force-pushed the backports_018_2a97f192e branch from 7fc706e to 7dd526f Compare July 28, 2021 23:02
@5tefan 5tefan requested a review from UdjinM6 July 28, 2021 23:04
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

👍

utACK

Copy link
Member

@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 for squash merge

@PastaPastaPasta PastaPastaPasta merged commit 1a29d78 into dashpay:develop Jul 29, 2021
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 3, 2021
…ional (dashpay#4297)

* Merge bitcoin#14771: test: Add BOOST_REQUIRE to getters returning optional

fa21ca0 test: Add BOOST_REQUIRE to getters returning optional
            (MarcoFalke)

Pull request description:

  Usually the returned value is already checked for equality, but for
sanity we might as well require that the getter successfully returned.

* Ports [[nodiscard]] portion of bitcoin#14771
@5tefan 5tefan deleted the backports_018_2a97f192e branch August 12, 2021 06:11
gades pushed a commit to cosanta/cosanta-core that referenced this pull request May 24, 2022
…ional (dashpay#4297)

* Merge bitcoin#14771: test: Add BOOST_REQUIRE to getters returning optional

fa21ca0 test: Add BOOST_REQUIRE to getters returning optional
            (MarcoFalke)

Pull request description:

  Usually the returned value is already checked for equality, but for
sanity we might as well require that the getter successfully returned.

* Ports [[nodiscard]] portion of bitcoin#14771
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