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

Merge v2 into master #1731

Merged
merged 267 commits into from
Oct 14, 2020
Merged

Merge v2 into master #1731

merged 267 commits into from
Oct 14, 2020

Conversation

johanbrandhorst
Copy link
Collaborator

Draft change of merging v2 into master. Probably have to freeze v2 and master from now until I can merge this. I'm going to come back to this and update things that are missing/incorrect before we can make the release.

Fixes #1223

johanbrandhorst and others added 30 commits April 29, 2020 09:26
As previously attempted on v1, so it shall be on v2.
We want to minimize the public API surface to more easily
allow changes that won't break users.
The token belongs to a user with no membership anywhere,
and it is limited to only reading github packages.
Replace with internal copy of CamelCase
Following on from #1242, this replace the StreamError
with a status.Status type. Also, remove the ability to configure
the stream error handler. The existing handler was specific to the
old type, and we can add something better back in later
if necessary.

Fixes #1098
… parameters (#1267)

* Added function nestedQueryParams with map[string] parameter for keeping track of and detecting circular references. Added test TestMessageToQueryParametersRecursive for testing gracefully handling of circular references between messages. See issue #1167

* Code-review change requests accepted

* More missed circle references changed to cycle

Fixes #1167
* httpbody in stream

* httpbody contenttype in stream response

* compare content type in test

* Update examples/internal/integration/integration_test.go

Co-authored-by: Johan Brandhorst <[email protected]>

* httpbody readme update

Co-authored-by: Johan Brandhorst <[email protected]>
Use half of the Circle runners resources.
All unary and streaming error responses are now
handled with a single function each,
and are configured in a single place.
@codecov-io
Copy link

Codecov Report

Merging #1731 into master will increase coverage by 5.36%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1731      +/-   ##
==========================================
+ Coverage   53.38%   58.74%   +5.36%     
==========================================
  Files          42       33       -9     
  Lines        3902     3624     -278     
==========================================
+ Hits         2083     2129      +46     
+ Misses       1562     1237     -325     
- Partials      257      258       +1     
Impacted Files Coverage Δ
internal/codegenerator/parse_req.go 100.00% <ø> (ø)
internal/descriptor/grpc_api_configuration.go 27.27% <ø> (ø)
internal/descriptor/openapi_configuration.go 30.00% <ø> (ø)
internal/descriptor/registry.go 64.22% <ø> (ø)
internal/descriptor/services.go 74.48% <ø> (ø)
internal/descriptor/types.go 50.44% <ø> (ø)
internal/httprule/compile.go 100.00% <ø> (ø)
internal/httprule/parse.go 85.71% <ø> (ø)
internal/httprule/types.go 100.00% <ø> (ø)
...-gen-grpc-gateway/internal/gengateway/generator.go 40.54% <ø> (-0.03%) ⬇️
... and 47 more

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 90c3950...d4e62de. Read the comment docs.

@ivucica
Copy link
Collaborator

ivucica commented Oct 10, 2020

Everything else is now green except CLA for @kentdotn and @odsod. Manual check by entering the username also gives me a red warning.

Kent, did you use the exact same username to submit the CLA?

FTR the emails that PR tool sees are [email protected] and [email protected] [mangled] and the usernames as above. I will open an internal bug for @kentdotn straight away.

@odsod
Copy link
Contributor

odsod commented Oct 10, 2020

I've signed the CLA just now - should be all good on my end!

@ivucica
Copy link
Collaborator

ivucica commented Oct 10, 2020

I believe the issue is that PR #1446 had a gmail.com address for @kentdotn, while the CLA bot sees the default github.com noreply address.

For @odsod in PR #1504, there was an einride.tech address.

The new patches in this PR have different addresses for some reason? I'll file this as an edge case that could be supported.

@ivucica
Copy link
Collaborator

ivucica commented Oct 10, 2020

@googlebot please be nice and recheck (though I fully expect you'll fail).

I'd like to let authors of the bot take a look at this by Tuesday afternoon GMT, and after that I'll flip the flag manually. The CLAs were in place when the PRs were submitted (the bot said so!), the authors said they've signed CLAs here in this thread, hence it's reasonable to proceed with "this is fine".

@google-cla
Copy link

google-cla bot commented Oct 10, 2020

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.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@ivucica
Copy link
Collaborator

ivucica commented Oct 12, 2020

@kentdotn Looks like your registration with the gmail.com address has Github username https://github.com/kentdotn (with the prefix). I'm told that preventing this will be implemented in future.

Can you fix the username? I'm told you may need to revoke and re-sign your CLA. If you're asked about the username, make sure you enter just kentdotn.

@kentdotn
Copy link
Contributor

@googlebot I fixed it.

@google-cla
Copy link

google-cla bot commented Oct 12, 2020

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.

@kentdotn
Copy link
Contributor

@googlebot I consent.

@google-cla
Copy link

google-cla bot commented Oct 12, 2020

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.

2 similar comments
@google-cla
Copy link

google-cla bot commented Oct 12, 2020

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.

@google-cla
Copy link

google-cla bot commented Oct 12, 2020

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.

@kentdotn
Copy link
Contributor

@ivucica I've changed username field in Edit Contact Information page but It seems it doesn't fix the situation.
Do you know how to revoke the contract? I couldn't find such buttons or links in https://cla.developers.google.com/clas or Edit Contact Information.

@ivucica
Copy link
Collaborator

ivucica commented Oct 13, 2020

@kentdotn It's actually all green on CLA bot's side. Your change did the trick. What I don't understand is why, despite all signals being green, the final result is still 'needs author consent'. I'll file a new bug.

@googlebot run the check again, please.

@google-cla
Copy link

google-cla bot commented Oct 13, 2020

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.

@ivucica
Copy link
Collaborator

ivucica commented Oct 13, 2020

I'm flipping the bit.

@google-cla
Copy link

google-cla bot commented Oct 13, 2020

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.

@google-cla google-cla bot added cla: no and removed cla: yes labels Oct 13, 2020
@ivucica ivucica added cla: yes and removed cla: no labels Oct 13, 2020
@johanbrandhorst johanbrandhorst merged commit 26104fd into master Oct 14, 2020
@johanbrandhorst johanbrandhorst deleted the merge-v2 branch October 14, 2020 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v2 release planning