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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: fix jsdoc in Table.insert() #1370

Closed
wants to merge 2 commits into from
Closed

docs: fix jsdoc in Table.insert() #1370

wants to merge 2 commits into from

Conversation

nielm
Copy link

@nielm nielm commented May 18, 2021

JSdoc incorrectly had DeleteRowsOptions as options param to Table.insert()

Fixes #1369 馃

@nielm nielm requested review from a team as code owners May 18, 2021 07:51
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 18, 2021
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/nodejs-spanner API. label May 18, 2021
@nielm
Copy link
Author

nielm commented May 18, 2021

@hengfengli

@hengfengli hengfengli changed the title Fix jsdoc in Table.insert() fix: fix jsdoc in Table.insert() May 18, 2021
@hengfengli
Copy link
Contributor

hengfengli commented May 18, 2021

@nielm The conventionalcommits check fails. I think the commit message needs to follow a convention.

JSdoc incorrectly had DeleteRowsOptions as options param to Table.insert()
@nielm
Copy link
Author

nielm commented May 18, 2021

@nielm The conventionalcommits check fails. I think the commit message needs to follow a convention.

fixed

@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #1370 (fcc54a7) into master (ecaaf3b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #1370    +/-   ##
========================================
  Coverage   98.60%   98.60%            
========================================
  Files          23       23            
  Lines       21950    21964    +14     
  Branches     1238     1112   -126     
========================================
+ Hits        21644    21658    +14     
  Misses        297      297            
  Partials        9        9            
Impacted Files Coverage 螖
src/backup.ts 99.82% <100.00%> (+<0.01%) 猬嗭笍
src/database.ts 99.96% <100.00%> (+<0.01%) 猬嗭笍
src/instance.ts 100.00% <100.00%> (酶)
src/session.ts 100.00% <100.00%> (酶)
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 ecaaf3b...fcc54a7. Read the comment docs.

@nielm
Copy link
Author

nielm commented May 18, 2021

Any idea what causes the check failures and what I can do to fix them?

@hengfengli
Copy link
Contributor

@stephenplusplus Hi Stephen, do you have any idea why the lint fails? This PR doesn't touch backup.ts at all.

@zoercai zoercai added priority: p2 Moderately-important priority. Fix may not be included in next release. type: docs Improvement to the documentation for an API. labels May 19, 2021
@skuruppu
Copy link
Contributor

@bcoe did we change lint settings? There are so many lint failures that didn't exist before.

@bcoe
Copy link
Contributor

bcoe commented May 21, 2021

@skuruppu the library we rely on for linting changed these rules in a minor version, so we'll need to update and run --fix locally.

@hengfengli
Copy link
Contributor

@bcoe It's strange. I ran npm run lint and npm run fix locally. I got no errors. But the lint CI check fails. Do you have any idea why this is happening?

@hengfengli
Copy link
Contributor

I got the output:

> @google-cloud/spanner@5.7.0 lint
> gts check

version: 15

The version is different from the CI check (version: 14). Is this an issue?

@skuruppu
Copy link
Contributor

@nielm I think I know what the issue is. I've had the same trouble for a long time as well.

When we run npm run fix locally, it makes a whole of of lint changes that are not compatible with the settings on GithubActions. So I usually run the command and revert all changes that are not related to the code I touched. It's a pain but will help you get this merged.

@hengfengli
Copy link
Contributor

@skuruppu The issue is that even with the original 1-line change (without any lint fixes), the lint check fails as well. That's why @bcoe suggested to run npm run fix.

@laljikanjareeya
Copy link
Contributor

@skuruppu @hengfengli I think we should create separate PR to fix the lint. I will create it shortly.

@laljikanjareeya laljikanjareeya changed the title fix: fix jsdoc in Table.insert() docs: fix jsdoc in Table.insert() May 21, 2021
@hengfengli
Copy link
Contributor

Since a PR #1371 to fix all doc issues has been created, I think this PR can be closed.

@skuruppu skuruppu closed this Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. cla: yes This human has signed the Contributor License Agreement. priority: p2 Moderately-important priority. Fix may not be included in next release. type: docs Improvement to the documentation for an API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect jsdoc for Table.insert()
7 participants