Skip to content

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented Aug 2, 2021

No description provided.

@kwvg kwvg marked this pull request as draft August 2, 2021 17:47
@kwvg kwvg force-pushed the fuzz branch 4 times, most recently from ff7a148 to 27ec275 Compare August 5, 2021 14:44
@kwvg kwvg marked this pull request as ready for review August 5, 2021 14:44
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.

partial review, almost everything looks good

@github-actions
Copy link

github-actions bot commented Aug 6, 2021

This pull request has conflicts, please rebase.

@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Aug 6, 2021

See https://github.com/PastaPastaPasta/dash/tree/pr_4312 for my rebased branch. (pretty trivial rebase)

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.

Comment on lines +285 to +295
- stage: test
env: >-
HOST=x86_64-unknown-linux-gnu
PACKAGES="clang llvm python3 libssl1.0-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev"
NO_DEPENDS=1
RUN_UNIT_TESTS=false
RUN_FUNCTIONAL_TESTS=false
RUN_FUZZ_TESTS=true
GOAL="install"
BITCOIN_CONFIG="--disable-wallet --disable-bench --with-utils=no --with-daemon=no --with-libs=no --with-gui=no --enable-fuzz --with-sanitizers=fuzzer,address CC=clang CXX=clang++"

Copy link

Choose a reason for hiding this comment

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

would be nice to have a Gitlab CI job similar to this to make sure things work as expected

@UdjinM6
Copy link

UdjinM6 commented Aug 11, 2021

linter still complains

Good job! The Boost dependency "boost/test/unit_test_monitor.hpp" is no longer used.
Please remove it from EXPECTED_BOOST_INCLUDES in test/lint/lint-includes.sh

https://gitlab.com/dashpay/dash/-/jobs/1493947535#L57

@kwvg
Copy link
Collaborator Author

kwvg commented Aug 11, 2021

Read the "Good Job", didn't read the "remove from file", resolved!

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.

LGTM, 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 merging via merge commit

@@ -1,11 +1,7 @@
// Copyright (c) 2009-2015 The Bitcoin Core developers
// Copyright (c) 2009-2018 The Bitcoin Core developers
Copy link
Member

Choose a reason for hiding this comment

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

nit: this shouldn't have been changed

@PastaPastaPasta PastaPastaPasta merged commit 90e7119 into dashpay:develop Aug 11, 2021
@UdjinM6 UdjinM6 added this to the 18 milestone Aug 11, 2021
@kwvg kwvg deleted the fuzz branch July 18, 2023 11:39
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