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.

Sign up for GitHub

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

Jump to bottom

feat(api core): simplify from_rfc3339 methods #8742

Closed
wants to merge 269 commits into from
Closed

feat(api core): simplify from_rfc3339 methods #8742

wants to merge 269 commits into from

Conversation

emar-kar
Copy link
Contributor

@emar-kar emar-kar commented Jul 24, 2019 •
edited
Loading

Combining methods from_rfc3339 and from_rfc3339_nanos as requested in #7676.
from_rfc3339_nanos method marked as deprecated and refers to the new implementation.

Closes: #7676.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 24, 2019
@emar-kar emar-kar marked this pull request as ready for review July 24, 2019 13:34
@emar-kar emar-kar requested a review from busunkim96 as a code owner July 24, 2019 13:34
@tseaver tseaver added api: core kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jul 24, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 24, 2019
tseaver
tseaver suggested changes Jul 24, 2019
View reviewed changes
Copy link
Contributor

@tseaver tseaver left a comment

Choose a reason for hiding this comment

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

In addition to deprecation (for benefit of hypothetical external clients), this PR needs remove all use of the _nanos version in our codebase (we don't want to be emitting deprecation warnings ourselves).

tseaver
tseaver suggested changes Jul 24, 2019
View reviewed changes
Copy link
Contributor

@tseaver tseaver left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please fix the coverage and lint failures.

@AVaksman AVaksman added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 25, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 25, 2019
@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 26, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 26, 2019
@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 26, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 26, 2019
@crwilcox crwilcox self-requested a review August 22, 2019 16:10
@IlyaFaer IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 23, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 23, 2019
@IlyaFaer IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 28, 2019
@yoshi-kokoro yoshi-kokoro removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 28, 2019
@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 6, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 6, 2019
crwilcox
crwilcox reviewed Nov 7, 2019
View reviewed changes
api_core/google/api_core/datetime_helpers.py
regular expression.
"""
# Raise deprecation warnings for things we want to go away.
warnings.warn(
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this warning then I am good

busunkim96 and others added 10 commits November 7, 2019 13:58
@busunkim96
feat(documentai): initial generation of documentai (googleapis#9623)
5137c03
@emar-kar
API Core: simplify methods
873a60d 
Two similar methods combined into one.
@emar-kar
Update test_datetime_helpers.py
796204c 
Re-write tests for the 'from_rfc3339_nanos' method.
@emar-kar
Update datetime_helpers.py
0503e92 
Fix for coverage and lint failures.
@emar-kar
Update datetime_helpers.py
dc4f896 
restore previous version with testing regex first + cosmetic chgs for the descriptions
@emar-kar
Update test_datetime_helpers.py
4bcf651 
added test to check the deprecation warning
@emar-kar
Update datetime_helpers.py
8151cae 
adjusted the condition for the 'from_rfc3339' method + warning fix
@emar-kar
Update datetime_helpers.py
447ed3e 
* deleted extra spaces
@emar-kar
update datetime_helpers
1fc8661 
* 'if-else' statement restored to throw an exception
* wrote additional tests to cover the exception
@emar-kar
Merge branch 'Methods-Combining' of https://github.com/q-logic/google…
93f8883 
…-cloud-python into Methods-Combining
@emar-kar emar-kar requested a review from a team November 8, 2019 08:15
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 8, 2019
@emar-kar emar-kar closed this Nov 8, 2019
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@emar-kar emar-kar mentioned this pull request Nov 8, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crwilcox crwilcox crwilcox approved these changes

@tseaver tseaver tseaver requested changes

@busunkim96 busunkim96 busunkim96 approved these changes

Assignees
No one assigned
Labels
api: core cla: no This human has *not* signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

API Core: Simplify from_rfc3339 methods