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.

Sign up for GitHub

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

Jump to bottom

Kms samples #496

Merged
merged 1 commit into from
Feb 1, 2017
Merged

Kms samples #496

merged 1 commit into from
Feb 1, 2017

Conversation

jerjou
Copy link
Contributor

@jerjou jerjou commented Jan 31, 2017

Working on tests, but figured I'd send this out first to start the review.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 31, 2017
lesv
lesv reviewed Jan 31, 2017
View reviewed changes
kms/src/main/java/com/example/Snippets.java

DestroyCryptoKeyVersionRequest destroyRequest = new DestroyCryptoKeyVersionRequest();

CryptoKeyVersion destroyed = kms.projects().locations().keyRings().cryptoKeys()
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably what our style guide says to do, but it seems like

kms.projects()
    .locations()
    .keyRings()
    .cryptoKeys()
    .cryptoKeyVersions()
    .destroy(cryptoKeyVersion, destroyRequest)
    .execute();

is more appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kind of like having most of the path in the same line, because it's just boilerplate to get to the main thing you care about (which is on the next line). That way the thing you care about is distinct
from the noise.

lesv
lesv suggested changes Jan 31, 2017
View reviewed changes
Copy link
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

On the whole, I really like this. We should start using kohsuke everywhere.

Feel free to ignore all but the "huh?".

kms/src/main/java/com/example/Snippets.java Outdated
for (Binding b : bindings) {
if (role.equals(b.getRole()) && b.getMembers().contains(member)) {
b.getMembers().remove(member);
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah. Right.
Since .remove can be optionally implemented, I wasn't sure if the List returned by getMembers() had it, so this was the contingency plan. Turns out it does, so this works, but I forgot I had the contingency here :-) Removed.

kms/README.md
and can see the available snippet commands with:

java -cp target/kms-samples-1.0.0-jar-with-dependencies.jar \
com.example.Snippets
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to mention any of the command line arguments here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After you run this, it'll show you the available arguments, which you can run, which will in turn show the available arguments for that subcommand.

I suppose having an actual example invocation wouldn't hurt..

@jerjou jerjou force-pushed the kms branch 3 times, most recently from 5f358a5 to c64baac Compare February 1, 2017 01:36
@jerjou
Copy link
Contributor Author

jerjou commented Feb 1, 2017

Updated - renamed a bunch of the command-line arguments to be more verbose, to be consistent with the naming from the API (and normalized some other inconsistent naming I had).

Having trouble with ADC while running tests 😫 It's mentioning a mysterious project id... sigh...

@lesv
Copy link
Contributor

lesv commented Feb 1, 2017

Once you figure out your mystery, you might wish to add the sample to the parent pom.xml to get tested.

@jerjou jerjou merged commit 363ceb1 into master Feb 1, 2017
@jerjou jerjou deleted the kms branch February 1, 2017 20:53
averikitsch pushed a commit that referenced this pull request Oct 27, 2022
@renovate-bot
chore(deps): update dependency com.google.cloud:libraries-bom to v21 (#…
c1468db 
…496)

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [com.google.cloud:libraries-bom](https://togithub.com/GoogleCloudPlatform/cloud-opensource-java) | `20.9.0` -> `21.0.0` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/21.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/21.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/21.0.0/compatibility-slim/20.9.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/21.0.0/confidence-slim/20.9.0)](https://docs.renovatebot.com/merge-confidence/) |

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-tasks).
@averikitsch averikitsch mentioned this pull request Oct 27, 2022
Merged
9 tasks
averikitsch pushed a commit that referenced this pull request Nov 1, 2022
@renovate-bot @averikitsch
chore(deps): update dependency com.google.cloud:libraries-bom to v21 (#…
7067ac9 
…496)

[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [com.google.cloud:libraries-bom](https://togithub.com/GoogleCloudPlatform/cloud-opensource-java) | `20.9.0` -> `21.0.0` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/21.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/21.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/21.0.0/compatibility-slim/20.9.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:libraries-bom/21.0.0/confidence-slim/20.9.0)](https://docs.renovatebot.com/merge-confidence/) |

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-tasks).
Shabirmean pushed a commit that referenced this pull request Nov 15, 2022
@renovate-bot
test(deps): update dependency com.google.truth:truth to v1.1.3 (#496)
602b73f
@Shabirmean Shabirmean mentioned this pull request Nov 15, 2022
Merged
9 tasks
Shabirmean pushed a commit that referenced this pull request Nov 15, 2022
@renovate-bot @Shabirmean
test(deps): update dependency com.google.truth:truth to v1.1.3 (#496)
2bfec95
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lesv lesv lesv approved these changes

Assignees

@lesv lesv

Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jerjou @lesv @googlebot