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 file reorganization #1986

Merged
merged 4 commits into from
Jun 22, 2022
Merged

Blockchain file reorganization #1986

merged 4 commits into from
Jun 22, 2022

Conversation

acolytec3
Copy link
Contributor

No description provided.

@acolytec3
Copy link
Contributor Author

@holgerd77 How does this version look? Sorry, this whole land of renaming files while preserving history is brand new to me. I used Gitlens on VS Code locally and it looks like it's now retaining the history of the renamed blockchain.ts file now

@acolytec3 acolytec3 changed the title Take 5 Blockchain file reorganization Jun 22, 2022
@codecov
Copy link

codecov bot commented Jun 22, 2022

Codecov Report

Merging #1986 (7809371) into master (1bd5c4f) will decrease coverage by 5.42%.
The diff coverage is 76.21%.

Impacted file tree graph

Flag Coverage Δ
block 84.30% <ø> (?)
blockchain 81.03% <76.21%> (+0.02%) ⬆️
client 78.34% <ø> (?)
common 95.01% <ø> (?)
devp2p 82.64% <ø> (?)
ethash 90.81% <ø> (ø)
evm 42.62% <ø> (?)
statemanager 84.55% <ø> (?)
trie 73.72% <ø> (ø)
tx 92.17% <ø> (?)
util 87.03% <ø> (?)
vm 76.87% <ø> (ø)

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

@holgerd77
Copy link
Member

Thanks, that looks good, I guess GitHub won't show this any better. 🙂

Are you also adding a types.ts file and moving types, options,... in? That would be great!

Do we have anything else?

@acolytec3
Copy link
Contributor Author

Thanks, that looks good, I guess GitHub won't show this any better. slightly_smiling_face

Are you also adding a types.ts file and moving types, options,... in? That would be great!

Do we have anything else?

I moved the types from blockchain.ts into their own file in the package/src directory. Is there any reason to bring the consensus or genesisState types into the master types.ts file? It feels pretty well compartmentalized to me.

@holgerd77
Copy link
Member

I moved the types from blockchain.ts into their own file in the package/src directory. Is there any reason to bring the consensus or genesisState types into the master types.ts file? It feels pretty well compartmentalized to me.

Yeah, maybe we just leave there. Thanks, looks all good! 🙂

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.

LGTM

@holgerd77 holgerd77 merged commit 008c469 into master Jun 22, 2022
@holgerd77 holgerd77 deleted the take-5 branch June 22, 2022 18:01
@jochem-brouwer
Copy link
Member

It looks like git history of the files is lost here, did you use git mv here?

@acolytec3
Copy link
Contributor Author

I tried, and tried, and tried, and tried. 😅

@holgerd77
Copy link
Member

I tried, and tried, and tried, and tried. 😅

🙂 😂

Yes, impressively hard stuff. Strange sometimes.

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.

3 participants