[go: nahoru, domu]

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

refactor Pythonic sake #1077

Merged
merged 61 commits into from
Mar 17, 2020
Merged

refactor Pythonic sake #1077

merged 61 commits into from
Mar 17, 2020

Conversation

mgaitan
Copy link
Collaborator
@mgaitan mgaitan commented Feb 9, 2020

A general refactor to simplify common patterns a make a the codebase a bit more pythonic. Flatten code, reduce complexity, less lambdas, and more

I've also augmented tests (and simplify them using pytest helpers) in few cases.

@tburrows13 tburrows13 added needs-more-info Waiting for submitter's reply, feedback, updates,... refactor Does not affect the end user at all i.e. making code easier to read or PEP8 compliant. labels Feb 24, 2020
@mgaitan
Copy link
Collaborator Author
mgaitan commented Feb 26, 2020

@tburrows13 thanks for the careful review. I'm in the middle of a trip now, but gonna update this following your comment ASAP, probably during the next wekeend.

@tburrows13 tburrows13 added this to the Release v2.0.0 milestone Feb 26, 2020
@tburrows13
Copy link
Collaborator

Sure, no hurry

@mgaitan mgaitan requested a review from tburrows13 March 5, 2020 04:35
@mgaitan
Copy link
Collaborator Author
mgaitan commented Mar 6, 2020

this is ready for a new review/merge .

@mgaitan mgaitan requested a review from Zulko March 6, 2020 13:00
@tburrows13 tburrows13 removed the needs-more-info Waiting for submitter's reply, feedback, updates,... label Mar 9, 2020
@tburrows13 tburrows13 removed the review-needed PRs which need to be reviewed (further) by project maintainers. label Mar 9, 2020
@mgaitan
Copy link
Collaborator Author
mgaitan commented Mar 12, 2020

So, could we merge this or an extra review is needed?

@mgaitan
Copy link
Collaborator Author
mgaitan commented Mar 17, 2020

@Zulko could you check and merge this one? It's blocking the other refactors proposed.

Alternatively, give me permission on the repo. (I'll use it with care, promise)

@tburrows13 tburrows13 merged commit f22e663 into Zulko:master Mar 17, 2020
@tburrows13
Copy link
Collaborator

I've got no issues with merging, so I'll go ahead and do it. I'd happily support giving @mgaitan the Collaborator role as well. I think if you merge the current master branch into your other PRs, then they'll be sorted now.

@tburrows13 tburrows13 added the tests Related to individual tests in the test suite or running the test suite. label Mar 17, 2020
@mgaitan
Copy link
Collaborator Author
mgaitan commented Mar 17, 2020

excellent, thanks!

@tburrows13
Copy link
Collaborator

Sure! I've been away this last weekend, but now that my university term has finished, I'm keen to get v1.0.2 out within a few days. Feel free to comment on any PRs that you think would be suitable and I can add them to the milestone.

@Zulko
Copy link
Owner
Zulko commented Mar 17, 2020

Thank you so much guys for taking care of the library! I am a bit out of touch but super happy to see that!

Martin I sent you a collaborator invitation, maybe add your name in the maintainers list in the Readme? Maybe also mark Tom and yourself as main maintainers, since you are currently the most up-to-date on the library?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Does not affect the end user at all i.e. making code easier to read or PEP8 compliant. tests Related to individual tests in the test suite or running the test suite.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants