-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add produce_eko_photon to evolven3fit_new #1778
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks ok but please instead of branching in construct_eko_cards
, just make another function for the photon. At the end in construct_eko_cards
you are doing a lot of different things so it is probably better to have another function. All the things that instead are in common you can put in yet another function to be called in both cases.
Actually the branching |
I agree with @andreab1997 I think it is better to have different functions (and then lift some of the common functionality to wrappers) rather than so many branches. |
@andreab1997 @scarlehoff Now I have separated the functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good for me!
PR for adding
produce_eko_photon
inevolven3fit_new