Uh oh!
There was an error while loading. Please reload this page.
ProjectOptions (public to private) & Project struct (org related fields) fixes - #2111
Conversation
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
DariuszPorowski
commented
Sep 28, 2021
@googlebot I fixed it. |
Codecov Report
@@ Coverage Diff @@## master #2111 +/- ##
=======================================
Coverage 97.77% 97.77% =======================================
Files 111 111 Lines 9934 9934 =======================================
Hits 9713 9713 Misses 154 154 Partials 67 67
Continue to review full report at Codecov.
|
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @DariuszPorowski !
LGTM.
Awaiting second LGTM before merging.
This is technically a Breaking API change... however, I'm tempted to declare this one as non-breaking simply because what we are changing was already broken... so in theory, any attempts to use the prior version would have failed.
Thoughts? Is it OK to NOT declare this PR as a breaking APi change (and therefore not have to bump the major version number of this repo at the next release due to this PR alone)?
@willnorris - what is your opinion?
willnorris
commented
Sep 28, 2021
Even though this was never working as intended, it's a breaking change in the Go API in that if anyone was calling it, their code would no longer compile now. And since it's been in the wild for 3 years, I feel like it would be more responsible to treat it as a breaking change. |
DariuszPorowski
commented
Sep 29, 2021
@gmlewis I agree with @willnorris |
cpanato
left a comment
There was a problem hiding this comment.
lgtm
we need to document that in the release notes
gmlewis
commented
Dec 3, 2021
Thank you, @cpanato - merging. |
This fixes#2110
I've done UATs as well, and all API calls looked good: