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

REF: collect parser methods by function #882

Merged
merged 3 commits into from
Feb 26, 2019

Conversation

jbrockmendel
Copy link
Contributor

The methods within parser are a bit of a jumble. This separates out two sections for methods that a) act on individual tokens and b) are for creating the datetime output after the string is fully parsed.

No hurry on this if it would cause rebasing conflicts elsewhere.

Methods are unchanged, with the exception of a long line in a comment in to_decimal that is wrapped.

@jbrockmendel
Copy link
Contributor Author

failed build is pypy, looks unrelated

Copy link
Member

@pganssle pganssle left a comment

Choose a reason for hiding this comment

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

Yeah I think this is a nicer arrangement for the code.

Can you add a misc item to the changelog?

@jbrockmendel
Copy link
Contributor Author

Can you add a misc item to the changelog?

Done. Is there a better word than "Fixed" in this context?

@pganssle
Copy link
Member

@jbrockmendel I often go with "Patch by ..." when contributing to CPython, but no need to worry. For the moment, these MISC things don't actually generate entries directly in the changelog (though I'm thinking maybe that should change).

jbrockmendel and others added 3 commits February 26, 2019 15:04
This is a minor refactoring to improve the code organization.
This is merged in the branch for PR dateutil#882 because PR dateutil#735 was
accidentally merged without a changelog.
@pganssle pganssle merged commit 5ddf09d into dateutil:master Feb 26, 2019
@jbrockmendel jbrockmendel deleted the send_upstream branch February 26, 2019 23:25
@pganssle pganssle mentioned this pull request Nov 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants