-
Notifications
You must be signed in to change notification settings - Fork 725
Split main.cpp into net_processing/validation as per upstream #1098
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
Conversation
… opportunities: * Divide main.cpp into validation.cpp and net_processing.cpp * Move class/struct definitions where they should be * Confirmed tests compile and run correctly
|
This was unexpected, effort really appreciated 👌 . Yeah, it will need some cleanup and updates but great man 👍 . Will review it properly and most likely give you a hand with the updates after 4.0. |
|
Hi, now that 4.0 is just about to be released, can continue moving forward with this 🤘 . First step would be plan this big PR division into a subset of smaller and focalized PRs on top of 4.0, and slowly grow from there to this point, otherwise this will hardly pass the review process. |
|
Agreed.
I think the way forward would be creating a skeleton set of
source/headers for validation/net_processing, then using my PR as a
loose guide as we bring the appropriate functions across.. although feel
free to suggest as well..
james
…On 17/12/2019 10:19 pm, Matias Furszyfer wrote:
Hi, now that 4.0 is just about to be released, can continue moving
forward with this 🤘 .
First step would be plan this big PR division into a subset of smaller
and focalized PRs on top of 4.0, and slowly grow from there to this
point, otherwise this will hardly pass the review process.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#1098?email_source=notifications&email_token=ADL537UWMLOS4YIPIHDMV23QZDNYVA5CNFSM4JJV4LT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHCQO3I#issuecomment-566560621>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADL537W5OPPERJM3JE5IK43QZDNYVANCNFSM4JJV4LTQ>.
|
|
Hmm, that would most likely end up being still pretty big. Would start porting/moving the isolated code (class by class, method by method). The one who have no modifications and possibly just a single call. Each of them in a separated PR. A quick example is the Slowly but steady cleaning the In this way, we will be able to always move forward merging PR by PR. Doing baby, well reviewed, steps. |
|
Have prepared a PR, mainly to check if this is headed in the right direction: Using Bitcoin 0.14 to 'steer' where required. |
|
is anyone opposed to closing this? No advances for a long period of time. |
|
Yep. closing... |
This won't be able to be used directly owing to the upcoming 4.0 release and associated change freeze (also work that has taken place in the meantime), however this will allow future upstream porting to be done with ease. Enjoy.