+
Skip to content

Update hook_id to int64 #1837

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

Closed
wants to merge 1 commit into from
Closed

Update hook_id to int64 #1837

wants to merge 1 commit into from

Conversation

lindluni
Copy link
Contributor

The hook_id field of the Audit Log API is not a string. It is represented as a number, updated the HookID to int64 to match other fields of type number.

Signed-off-by: Brett Logan lindluni@github.com

@google-cla
Copy link

google-cla bot commented Mar 31, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Mar 31, 2021
The hook_id field is not a string it represented as
a number, updated the HookID to int64.

Signed-off-by: Brett Logan <lindluni@github.com>
@google-cla
Copy link

google-cla bot commented Mar 31, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@codecov
Copy link

codecov bot commented Mar 31, 2021

Codecov Report

Merging #1837 (7d32ff9) into master (3130b78) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1837   +/-   ##
=======================================
  Coverage   97.56%   97.56%           
=======================================
  Files         104      104           
  Lines        6602     6602           
=======================================
  Hits         6441     6441           
  Misses         87       87           
  Partials       74       74           
Impacted Files Coverage Δ
github/orgs_audit_log.go 100.00% <ø> (ø)

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 3130b78...7d32ff9. Read the comment docs.

@lindluni
Copy link
Contributor Author

Working on the CLA

@lindluni
Copy link
Contributor Author

lindluni commented Mar 31, 2021

Pagination is also broken for the audit-log API, I've opened an issue internally as the current Pagination mechanism for this endpoint is completely different from all other endpoints. So while this change is good, it wont fix the actual audit log API. I'll open an issue here to track that problem as well.

@gmlewis
Copy link
Collaborator

gmlewis commented Mar 31, 2021

Thank you, @lindluni !
This is a small enough change that if you need this ASAP, we could close this PR and I could open a new one (until you get your CLA working). It's your call.

@lindluni
Copy link
Contributor Author

lindluni commented Mar 31, 2021

Thank you, @lindluni !
This is a small enough change that if you need this ASAP, we could close this PR and I could open a new one (until you get your CLA working). It's your call.

I'm happy to have you submit a separate PR. I'm just waiting on a response from legal. Appreciate the help. I'll get an issue open here for the pagination problem as well to track it externally, once I get an update from the engineering team I'll update it here as well.

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.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载