Skip to content
This repository was archived by the owner on Dec 8, 2021. It is now read-only.

fix: correct range validation in ParseRfc3339#220

Merged
coryan merged 1 commit intogoogleapis:v0.22.xfrom
coryan:prepare-v0.22.1-patch
Mar 10, 2020
Merged

fix: correct range validation in ParseRfc3339#220
coryan merged 1 commit intogoogleapis:v0.22.xfrom
coryan:prepare-v0.22.1-patch

Conversation

@coryan
Copy link
Copy Markdown
Contributor

@coryan coryan commented Mar 10, 2020

It was not accepting 59 as a valid minute, this bug was introduced in
the clang-tidy cleanup, and our tests validate that things outside the
valid ranges are correctly reported as errors, but not that things
exactly at the boundary conditions are reported as valid. Sigh.

I ran the clang-tidy build for g-c-cpp and g-c-cpp-spanner against
these changes, the build passed in both cases.


This change is Reviewable

It was not accepting 59 as a valid minute, this bug was introduced in
the clang-tidy cleanup, and our tests validate that things *outside* the
valid ranges are correctly reported as errors, but not that things
*exactly* at the boundary conditions are reported as valid. Sigh.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 10, 2020
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 10, 2020

Codecov Report

Merging #220 into v0.22.x will decrease coverage by 0.01%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           v0.22.x     #220      +/-   ##
===========================================
- Coverage    96.28%   96.26%   -0.02%     
===========================================
  Files           97       97              
  Lines         4443     4447       +4     
===========================================
+ Hits          4278     4281       +3     
- Misses         165      166       +1
Impacted Files Coverage Δ
google/cloud/internal/parse_rfc3339.cc 98.88% <100%> (ø) ⬆️
google/cloud/internal/parse_rfc3339_test.cc 92% <80%> (-0.71%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1f5f01...ea8850c. Read the comment docs.

@coryan coryan marked this pull request as ready for review March 10, 2020 13:30
Copy link
Copy Markdown
Contributor

@devbww devbww left a comment

Choose a reason for hiding this comment

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

Why does this look like a repeat of #219. Should not it be more like "create new release"?

@devbww
Copy link
Copy Markdown
Contributor

devbww commented Mar 10, 2020

Oh ... I see ... it is a cherrypick into a branch.

@coryan coryan merged commit 6b1cd34 into googleapis:v0.22.x Mar 10, 2020
@coryan coryan deleted the prepare-v0.22.1-patch branch March 10, 2020 15:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants