Skip to content

ci: Add --enable-werror to arm and c++17 builds (and fix all issues found via these builds)#4203

Merged
UdjinM6 merged 16 commits intodashpay:developfrom
UdjinM6:pr4196
Jul 16, 2021
Merged

ci: Add --enable-werror to arm and c++17 builds (and fix all issues found via these builds)#4203
UdjinM6 merged 16 commits intodashpay:developfrom
UdjinM6:pr4196

Conversation

@UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Jun 23, 2021

No description provided.

@UdjinM6 UdjinM6 added this to the 17.1 milestone Jun 23, 2021
@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Jul 8, 2021

Is this ready?

@UdjinM6
Copy link
Author

UdjinM6 commented Jul 8, 2021

Rebased and fixed a couple of new issues. Will require another rebase after #4223 though.

@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Jul 13, 2021

Ready to review?

@UdjinM6 UdjinM6 marked this pull request as ready for review July 13, 2021 16:41
UdjinM6 added 16 commits July 13, 2021 19:43
@UdjinM6
ci: Add --enable-werror to arm and c++17 builds
b797fbd 
Kind of similar to changes in 18145
@UdjinM6
Remove/fix unused variables
a18707e
@UdjinM6
Remove duplicate rescanblockchain declaration
0f17a13 
leftover from 13899
@UdjinM6
Hold locks in CHDChain::swap()
934826d
@UdjinM6
Fix locks in EnforceBestChainLock()
d2a1bab
@UdjinM6
Fix locks in CDKGSession
3e95d59
@UdjinM6
Drop external protection of pending messages in CDKGSessionHandler
a65a466
@UdjinM6
Hold the right lock in AskNodesForLockedTx
cf38963
@UdjinM6
Check for locks in more places in CSigSharesManager
66f3f4f
@UdjinM6
Fix CConnman related locks
cc601e8
@UdjinM6
Fix locks in CMasternodeSync
64d6e57
@UdjinM6
Fix mempool locks
8934a65
@UdjinM6
Remove duplicates/add checks where needed for cs_main/cs_wallet
4aa8689
@UdjinM6
Initialize firstNodeId
69a72ea
@UdjinM6
Initialize nBytes in CreateTransaction
b472a70
@UdjinM6
Add more compile time checking for cs_main runtime locking assertions
d1ff298
@UdjinM6
Copy link
Author

UdjinM6 commented Jul 13, 2021

It was looking good before recent merges to develop and I think there were no breaking changes so it should be ready... but I rebased it one more time just in case, let's see :)

@PastaPastaPasta PastaPastaPasta self-requested a review July 13, 2021 16:54
PastaPastaPasta
PastaPastaPasta approved these changes Jul 13, 2021
View reviewed changes
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

@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Jul 13, 2021

CI happy :)

@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Jul 15, 2021

merge?

@UdjinM6
Copy link
Author

UdjinM6 commented Jul 15, 2021 •
edited
Loading

I think it conflicts with #4247 and also, it requires #4254. I'd like to rebase it once again once these two are merged to make sure everything is clean. Changed my mind :)

@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Jul 15, 2021

It'd be very nice if we had the bot that bitcoin has to identify conflicting PRs

@UdjinM6
Copy link
Author

UdjinM6 commented Jul 16, 2021

Ok, so #4254 is merged now. But #4247 isn't quite ready yet it seems, so I'm just going to merge this one first instead.

(I verified locally that it compiles with --enable-werror after rebasing it on top of develop).

@UdjinM6 UdjinM6 merged commit 886024b into dashpay:develop Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

18

Development

Successfully merging this pull request may close these issues.

2 participants

@UdjinM6 @PastaPastaPasta