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

fix: Noop in case there is no change in autocommit value for setAutocommit() method#2662

Merged
ankiaga merged 5 commits intogoogleapis:mainfrom
ankiaga:fix-autocommit-noop
Oct 10, 2023
Merged

fix: Noop in case there is no change in autocommit value for setAutocommit() method#2662
ankiaga merged 5 commits intogoogleapis:mainfrom
ankiaga:fix-autocommit-noop

Conversation

@ankiaga
Copy link
Copy Markdown
Contributor

@ankiaga ankiaga commented Oct 9, 2023 •
edited
Loading

Noop in case there is no change in autocommit value for setAutocommit() method

@ankiaga ankiaga requested a review from a team October 9, 2023 08:22
@product-auto-label product-auto-label Bot added size: s Pull request size is small. api: spanner Issues related to the googleapis/java-spanner API. labels Oct 9, 2023
@olavloite
Copy link
Copy Markdown
Collaborator

olavloite commented Oct 9, 2023

A couple of general comments on the pull request itself:

  1. Try to make the first line of the commit message 80 characters or less. Alternatively; manually edit the title of the pull request so it's not broken into a part that is in the title and a part that is in the description.
  2. The semantic prefix should be written using all lower-case letters (so fix: instead of Fix:)
  3. Add a description to the pull request. GitHub creates this automatically if you format your commit message as follows:
fix: my pull request header

My Pull Request description that may span
multiple lines. If your pull request also fixes a
specific issue, then you should end your commit
message with a suffix that says 'Fixes #github_issue_number'.

Fixes #2662

@ankiaga ankiaga changed the title Fix: Noop in case there is no change in autocommit value for setAutocommit() method fix: Noop in case there is no change in autocommit value for setAutocommit() method Oct 9, 2023
@product-auto-label product-auto-label Bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Oct 9, 2023
olavloite
olavloite approved these changes Oct 9, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of minor nits.

Comment thread google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java Outdated
subject.execute(Statement.of("begin transaction"));

subject.setAutocommit(false);
fail("Cannot set autocommit while in a temporary transaction");
Copy link
Copy Markdown
Collaborator

@olavloite olavloite Oct 9, 2023

Choose a reason for hiding this comment

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

nit: use assertThrows(..) for this.

I know that there are examples in this test class that use this setup with a try-fail block. Those are however left-overs from when this library had to support Java 7, and we did not have support for lambda expressions.

Copy link
Copy Markdown
Contributor Author

@ankiaga ankiaga Oct 9, 2023

Choose a reason for hiding this comment

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

Done

Comment thread google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java
@ankiaga ankiaga added the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 10, 2023
@gcf-owl-bot gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Oct 10, 2023
@ankiaga ankiaga requested a review from a team October 10, 2023 10:17
@ankiaga ankiaga merged commit 9f51b64 into googleapis:main Oct 10, 2023
@ankiaga ankiaga deleted the fix-autocommit-noop branch October 10, 2023 10:55
@release-please release-please Bot mentioned this pull request Oct 10, 2023
Merged
gcf-merge-on-green Bot pushed a commit that referenced this pull request Oct 11, 2023
@release-please
chore(main): release 6.50.1 (#2667)
77b006d 
🤖 I have created a release *beep* *boop*
---


## [6.50.1](https://togithub.com/googleapis/java-spanner/compare/v6.50.0...v6.50.1) (2023-10-11)


### Bug Fixes

* Noop in case there is no change in  autocommit value for setAutocommit() method ([#2662](https://togithub.com/googleapis/java-spanner/issues/2662)) ([9f51b64](https://togithub.com/googleapis/java-spanner/commit/9f51b6445f064439379af752372a3490a2fd5087))


### Dependencies

* Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.17.0 ([#2660](https://togithub.com/googleapis/java-spanner/issues/2660)) ([96b9dd6](https://togithub.com/googleapis/java-spanner/commit/96b9dd6b6a0ee7b1a0a1cc58a8880a10799665e6))
* Update dependency commons-io:commons-io to v2.14.0 ([#2649](https://togithub.com/googleapis/java-spanner/issues/2649)) ([fa1b73c](https://togithub.com/googleapis/java-spanner/commit/fa1b73c1bf4700be5e8865211817e2bc7cc77119))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@olavloite olavloite olavloite approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: spanner Issues related to the googleapis/java-spanner API. size: m Pull request size is medium.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ankiaga @olavloite