Skip to content
This repository was archived by the owner on May 8, 2026. It is now read-only.

samples: subscription detachment#289

Merged
anguillanneuf merged 4 commits into
masterfrom
samples-detach
Sep 1, 2020
Merged

samples: subscription detachment#289
anguillanneuf merged 4 commits into
masterfrom
samples-detach

Conversation

@anguillanneuf

@anguillanneuf anguillanneuf commented Jul 15, 2020

Copy link
Copy Markdown
Contributor

Add Java sample for subscription detachment. The test can't be run yet because the feature is still being rolled out.

  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

FYI: A DetachSubscriptionRequest is used instead of SubscriptionName.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 15, 2020
@hannahrogers-google hannahrogers-google added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 15, 2020
@anguillanneuf anguillanneuf marked this pull request as ready for review September 1, 2020 18:52
@anguillanneuf anguillanneuf requested a review from a team September 1, 2020 18:52
@anguillanneuf anguillanneuf removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 1, 2020
.addPaths("dead_letter_policy.dead_letter_topic")
// A default of 5 is applied upon successful update.
.addPaths("dead_letter_policy.max_delivery_attempts")
.addPaths("dead_letter_policy")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This, like it's predecessor looks wrong - shouldn't this be a constant defined in the library?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Delivery attempt is not a constant defined in the library. In fact, it is best-effort only, and when no dead letter policy is attached, users won't be able to access delivery attempts.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Understood - if it's understood as something important and understood as something special - it should be defined in the library in some way.

@codecov

codecov Bot commented Sep 1, 2020

Copy link
Copy Markdown

Codecov Report

Merging #289 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #289   +/-   ##
=========================================
  Coverage     79.21%   79.21%           
  Complexity      318      318           
=========================================
  Files            21       21           
  Lines          2892     2892           
  Branches        155      155           
=========================================
  Hits           2291     2291           
  Misses          536      536           
  Partials         65       65           

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 0d97689...513c4f4. Read the comment docs.

@anguillanneuf anguillanneuf merged commit 785981a into master Sep 1, 2020
@anguillanneuf anguillanneuf deleted the samples-detach branch September 1, 2020 19:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants