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

Add support for string roles & roles in reads #346

Merged
merged 1 commit into from
Apr 27, 2021
Merged

Add support for string roles & roles in reads #346

merged 1 commit into from
Apr 27, 2021

Conversation

konne88
Copy link
Contributor

@konne88 konne88 commented Apr 8, 2021

This addresses bugs:
#344
#345

@konne88 konne88 requested a review from antoninbas April 8, 2021 19:53
@konne88
Copy link
Contributor Author

konne88 commented Apr 20, 2021

Antonin, I just got the tests to pass. Can you review. Thanks!

docs/v1/P4Runtime-Spec.mdk Outdated Show resolved Hide resolved
docs/v1/P4Runtime-Spec.mdk Outdated Show resolved Hide resolved
docs/v1/P4Runtime-Spec.mdk Outdated Show resolved Hide resolved
docs/v1/P4Runtime-Spec.mdk Outdated Show resolved Hide resolved
Copy link
Member

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

could you squash the commits and include the following in the commit message body:

Fixes #344 
Fixes #345 

It will link the PR to these issues properly and close them automatically when the PR is merged. Thanks.

docs/v1/P4Runtime-Spec.mdk Show resolved Hide resolved
docs/v1/P4Runtime-Spec.mdk Show resolved Hide resolved
@konne88
Copy link
Contributor Author

konne88 commented Apr 22, 2021

I squashed the commits.

@antoninbas
Copy link
Member

@konne-google you have merged conflicts, as I had to merge a separate PR

Copy link
Member

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM

@konne88
Copy link
Contributor Author

konne88 commented Apr 26, 2021

@antoninbas , I don't think I can merge the request (The base branch restricts merging to authorized users.), could you do it for me? There is still a failing test, but I don't think I can fix it, it says:

docker: toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit.

@antoninbas
Copy link
Member

antoninbas commented Apr 26, 2021

@konne88 I restarted the job, I'll merge when it passes

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

3 participants