Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Sep 29, 2021

To avoid having all files at the top level ./src directory, start moving them to their respective sub directory according to #15732.

bloom currently depends on libconsensus (CTransaction, CScript, ...) and it is currently located in the libcommon. Thus, move it to src/common/. (libutil in src/util/ is for stuff that doesn't depend on libconsensus).

@maflcko
Copy link
Member Author

maflcko commented Sep 29, 2021

See also #22951 (which has a minor conflict in one adjacent line).

Copy link

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

Concept ACK fa762d0

@maflcko maflcko removed the P2P label Sep 30, 2021
@practicalswift
Copy link
Contributor

Concept ACK

1 similar comment
@naumenkogs
Copy link
Member

Concept ACK

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 30, 2021

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

Conflicts

No conflicts as of last run.

MarcoFalke added 3 commits October 5, 2021 11:08
-BEGIN VERIFY SCRIPT-
 # Move to directory
 mkdir                src/common
 git mv src/bloom.cpp src/common/
 git mv src/bloom.h   src/common/

 # Replace occurrences
 sed -i 's|\<bloom\.cpp\>|common/bloom.cpp|g' $(git grep -l 'bloom.cpp')
 sed -i 's|\<bloom\.h\>|common/bloom.h|g'     $(git grep -l 'bloom.h')
 sed -i 's|BITCOIN_BLOOM_H|BITCOIN_COMMON_BLOOM_H|g' $(git grep -l 'BLOOM_H')
-END VERIFY SCRIPT-
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

Code-review ACK fa2d611

TIL how to move files via scripted-diff (IIRC I tried once and failed, probably used "mv" rather than "git mv"...)

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Concept ACK.

return contains(MakeUCharSpan(stream));
return contains(stream);
Copy link
Member

Choose a reason for hiding this comment

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

fac303c

Why MakeUCharSpan became unused? What has been changed since 2ba4ddf from #23115?

Copy link
Member Author

Choose a reason for hiding this comment

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

There was simply no reason to add it. cc @fanquake

@maflcko maflcko requested a review from ryanofsky October 11, 2021 06:39
@maflcko maflcko requested a review from fanquake October 20, 2021 08:48
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Code review ACK fa2d611

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK fa2d611 - source shuffle starts now.

@fanquake fanquake merged commit c53e95f into bitcoin:master Oct 21, 2021
@maflcko maflcko deleted the 2109-srcCommon branch October 21, 2021 08:02
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 21, 2021
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants