Skip to content

Conversation

sethaustinx
Copy link
Contributor

To simplify review and address specific requests from #1270 (comment) I've made separate commits.

Also wording doesn't seem to match GitHub docs anymore.

If that's ok @gmlewis, I'd like to rename:

What do you think?

@google-cla
Copy link

google-cla bot commented Mar 23, 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 23, 2021
@sethaustinx
Copy link
Contributor Author

@googlebot I signed it!

@google-cla google-cla bot added cla: yes Indication that the PR author has signed a Google Contributor License Agreement. and removed cla: no labels Mar 23, 2021
@codecov
Copy link

codecov bot commented Mar 23, 2021

Codecov Report

Merging #1827 (5bfa385) into master (94eb482) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1827      +/-   ##
==========================================
+ Coverage   97.56%   97.59%   +0.02%     
==========================================
  Files         104      104              
  Lines        6602     6599       -3     
==========================================
- Hits         6441     6440       -1     
+ Misses         87       86       -1     
+ Partials       74       73       -1     
Impacted Files Coverage Δ
github/apps_marketplace.go 88.23% <100.00%> (+3.05%) ⬆️

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 94eb482...5bfa385. Read the comment docs.

@gmlewis
Copy link
Collaborator

gmlewis commented Mar 24, 2021

If that's ok @gmlewis, I'd like to rename:

What do you think?

Yes, those seem like reasonable changes (since we are already breaking the API).
Thank you, @sethaustinx . I'll take a look at your PR.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @sethaustinx !
LGTM.

Awaiting second LGTM before merging.

@gmlewis gmlewis added the Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). label Mar 24, 2021
@gmlewis gmlewis requested a review from wesleimp March 24, 2021 02:00
Copy link
Collaborator

@wesleimp wesleimp left a comment

Choose a reason for hiding this comment

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

💙

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 1, 2021

Thank you, @wesleimp !
Merging.

@gmlewis gmlewis merged commit ad42336 into google:master Apr 1, 2021
@sethaustinx
Copy link
Contributor Author

Awesome 🧡

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). cla: yes Indication that the PR author has signed a Google Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants