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

Blockchain reorganization #1980

Closed
wants to merge 2 commits into from
Closed

Blockchain reorganization #1980

wants to merge 2 commits into from

Conversation

acolytec3
Copy link
Contributor

Reorganizes blockchain to only do exports index.ts
blockchain class and its opts are moved to blockchain.ts

@codecov
Copy link

codecov bot commented Jun 21, 2022

Codecov Report

Merging #1980 (2c2a525) into master (1bd5c4f) will decrease coverage by 5.40%.
The diff coverage is 76.26%.

Impacted file tree graph

Flag Coverage Δ
block 84.30% <ø> (?)
blockchain 81.05% <76.26%> (+0.04%) ⬆️
client 78.34% <ø> (?)
common 95.01% <ø> (?)
devp2p 82.51% <ø> (?)
ethash 90.81% <ø> (ø)
evm 42.62% <ø> (?)
statemanager 84.55% <ø> (?)
trie 74.21% <ø> (+0.48%) ⬆️
tx 92.17% <ø> (?)
util 87.03% <ø> (?)
vm 76.87% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Ah, the blockchain.ts file has lost its history, this would need to be re-applied I guess.

So the procedure would be: do a first commit renaming index.ts to blockchain.ts. And then do a second creating and adding a new index.ts just with the exports.

Otherwise we would loose the whole index.ts file and change history.

Does this make sense or am I missing something?

@acolytec3
Copy link
Contributor Author

Ah, the blockchain.ts file has lost its history, this would need to be re-applied I guess.

So the procedure would be: do a first commit renaming index.ts to blockchain.ts. And then do a second creating and adding a new index.ts just with the exports.

Otherwise we would loose the whole index.ts file and change history.

Does this make sense or am I missing something?

I followed the path you suggested in #1981 and it looks like it did the same thing. Does it look any different to you?

@holgerd77
Copy link
Member

Did you move natively with git, so with git mv? 🤔

@acolytec3
Copy link
Contributor Author

Did you move natively with git, so with git mv? thinking

Ah, no, I didn't even realize that existed. Learn something new every day. Will close this PR and open a new one done under that approach.

@acolytec3 acolytec3 closed this Jun 22, 2022
@acolytec3 acolytec3 deleted the blockchain-reorg branch June 22, 2022 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants