feat: CommitStats in CommitResponse

[skuruppu]

If returnCommitStats = true in the CommitRequest, then CommitResponse will contain CommitStats.

[codecov]

Codecov Report

Merging #1254 (277002c) into master (a6d1aa2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1254   +/-   ##
=======================================
  Coverage   98.58%   98.58%           
=======================================
  Files          23       23           
  Lines       21633    21658   +25     
  Branches     1210     1213    +3     
=======================================
+ Hits        21327    21352   +25     
  Misses        297      297           
  Partials        9        9           

Impacted Files	Coverage Δ
src/table.ts	100.00% <100.00%> (ø)
src/transaction.ts	100.00% <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 a6d1aa2...c31cdfb. Read the comment docs.

[AVaksman]

Do we need unit tests for Table functions to confirm that returnCommitStats is accepted?

[skuruppu]

Do we need unit tests for Table functions to confirm that returnCommitStats is accepted?

I was curious about this. CodeCov gave 100% coverage for table.js so I wasn't sure. But I've now added tests to set returnCommitStats.

[skuruppu]

@olavloite @AVaksman I've been asked to not make this a breaking change. So the options for implementations are:

1. Make CommitOptions extend CallSettings.
2. Introduce a second method like commitWithOptions, insertWithOptions, etc. for all methods that needs to take in this option.

Could you advice me on which one of these is preferable?

[olavloite]

@olavloite @AVaksman I've been asked to not make this a breaking change. So the options for implementations are:

1. Make CommitOptions extend CallSettings.
2. Introduce a second method like commitWithOptions, insertWithOptions, etc. for all methods that needs to take in this option.

Could you advice me on which one of these is preferable?

Purely from a (Spanner) API perspective, I would prefer option 1 to prevent the API from being cluttered with a large number of ...WithOptions methods. I however don't know what has been agreed on a general level for all Google Cloud NodeJS API's regarding this, and therefore don't know what weight should be given to the argument given by @AVaksman that this would break the common practice.

[snippet-bot]

Here is the summary of changes.

You added 1 region tag.

• samples/get-commit-stats.js:18, tag spanner_get_commit_stats

---

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

• Refresh this comment

[skuruppu]

Based on the discussion in #1282 (comment), I reverted this to make it a non-breaking change.