Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Move functions from helpers.py to committee_helpers.py and epoch_processing_helpers.py #262

Merged
merged 3 commits into from
Feb 7, 2019

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Feb 6, 2019

What was wrong?

Too many functions in helpers.py, and the test_helpers.py is getting longer and longer.

How was it fixed?

No logic changes in this PR.

Note the dependencies are:

  • committee_helpers.py depends on helper.py.
  • epoch_processing_helpers depends on helper.py and committee_helpers.py.

/cc @pipermerriam @djrtwo @ChihChengLiang @NIC619 @jannikluhn

Any objection to this reorg? If no, I will continue to reorg test_helpers.py too.

Other thoughts on the file path reorg:

  1. There may be another new category attestation_helpers.py for the helper functions that related to Attestation and SlashableAttestation, but I'm inclined to leave it after Remove placeholder #250 and other in-progressing PR merged.
  2. We can move all the _helpers.py files into helpers folder, and remove the suffix.

Cute Animal Picture

pexels-photo-1260803

@hwwhww hwwhww added the eth2.0 label Feb 6, 2019
@djrtwo
Copy link
Contributor

djrtwo commented Feb 6, 2019

reorg lgtm

@pipermerriam
Copy link
Member

2. We can move all the `_helpers.py` files into `helpers` folder, and remove the suffix.

👍

  • helpers.attestations
  • helpers.committee

That makes a lot more sense to me than suffixing.

Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

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

Rubber stamp on the conceptual re-org. didn't actually browse the details of the diff.

@hwwhww hwwhww merged commit 2058916 into ethereum:master Feb 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants