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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate code from googleapis/nodejs-asset #2856

Merged
merged 115 commits into from
Nov 18, 2022
Merged

Conversation

ahrarmonsur
Copy link
Contributor

@ahrarmonsur ahrarmonsur requested a review from a team as a code owner November 16, 2022 21:04
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Nov 16, 2022
@snippet-bot
Copy link

snippet-bot bot commented Nov 16, 2022

Here is the summary of changes.

You are about to add 15 region tags.

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

@google-cla
Copy link

google-cla bot commented Nov 16, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Contributor

@ace-n ace-n left a comment

Choose a reason for hiding this comment

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

Can you add a workflow config to .github/workflows for this, so that the tests run?

Be sure to add the config to .github/workflows/workflows.json, too. ✅

@ahrarmonsur
Copy link
Contributor Author

The CI was failing due to lack of tests being carried over (the one mega-test in samples.test.js was marked as a quickstart test). However, it appears that one file was calling the tests for all the samples.

On @grayside's advice to unblock this PR, I have:

Context is in this conversation

jkwlui and others added 21 commits November 17, 2022 10:53
chore(deps): update dependency mocha to v6

This PR contains the following updates:

| Package | Type | Update | Change | References |
|---|---|---|---|---|
| mocha | devDependencies | major | `^5.2.0` -> `^6.0.0` | [homepage](https://mochajs.org/), [source](https://github.com/mochajs/mocha) |

---

### Release Notes

<details>
<summary>mochajs/mocha</summary>

### [`v6.0.0`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#&#8203;600--2019-02-18)

[Compare Source](https://github.com/mochajs/mocha/compare/v5.2.0...v6.0.0)

#### 🎉 Enhancements

-   [#&#8203;3726](https://github.com/mochajs/mocha/issues/3726): Add ability to unload files from `require` cache ([**@&#8203;plroebuck**](https://github.com/plroebuck))

#### 🐛 Fixes

-   [#&#8203;3737](https://github.com/mochajs/mocha/issues/3737): Fix falsy values from options globals ([**@&#8203;plroebuck**](https://github.com/plroebuck))
-   [#&#8203;3707](https://github.com/mochajs/mocha/issues/3707): Fix encapsulation issues for `Suite#_onlyTests` and `Suite#_onlySuites` ([**@&#8203;vkarpov15**](https://github.com/vkarpov15))
-   [#&#8203;3711](https://github.com/mochajs/mocha/issues/3711): Fix diagnostic messages dealing with plurality and markup of output ([**@&#8203;plroebuck**](https://github.com/plroebuck))
-   [#&#8203;3723](https://github.com/mochajs/mocha/issues/3723): Fix "reporter-option" to allow comma-separated options ([**@&#8203;boneskull**](https://github.com/boneskull))
-   [#&#8203;3722](https://github.com/mochajs/mocha/issues/3722): Fix code quality and performance of `lookupFiles` and `files` ([**@&#8203;plroebuck**](https://github.com/plroebuck))
-   [#&#8203;3650](https://github.com/mochajs/mocha/issues/3650), [#&#8203;3654](https://github.com/mochajs/mocha/issues/3654): Fix noisy error message when no files found ([**@&#8203;craigtaub**](https://github.com/craigtaub))
-   [#&#8203;3632](https://github.com/mochajs/mocha/issues/3632): Tests having an empty title are no longer confused with the "root" suite ([**@&#8203;juergba**](https://github.com/juergba))
-   [#&#8203;3666](https://github.com/mochajs/mocha/issues/3666): Fix missing error codes ([**@&#8203;vkarpov15**](https://github.com/vkarpov15))
-   [#&#8203;3684](https://github.com/mochajs/mocha/issues/3684): Fix exiting problem in Node.js v11.7.0+ ([**@&#8203;addaleax**](https://github.com/addaleax))
-   [#&#8203;3691](https://github.com/mochajs/mocha/issues/3691): Fix `--delay` (and other boolean options) not working in all cases ([**@&#8203;boneskull**](https://github.com/boneskull))
-   [#&#8203;3692](https://github.com/mochajs/mocha/issues/3692): Fix invalid command-line argument usage not causing actual errors ([**@&#8203;boneskull**](https://github.com/boneskull))
-   [#&#8203;3698](https://github.com/mochajs/mocha/issues/3698), [#&#8203;3699](https://github.com/mochajs/mocha/issues/3699): Fix debug-related Node.js options not working in all cases ([**@&#8203;boneskull**](https://github.com/boneskull))
-   [#&#8203;3700](https://github.com/mochajs/mocha/issues/3700): Growl notifications now show the correct number of tests run ([**@&#8203;outsideris**](https://github.com/outsideris))
-   [#&#8203;3686](https://github.com/mochajs/mocha/issues/3686): Avoid potential ReDoS when diffing large objects ([**@&#8203;cyjake**](https://github.com/cyjake))
-   [#&#8203;3715](https://github.com/mochajs/mocha/issues/3715): Fix incorrect order of emitted events when used programmatically ([**@&#8203;boneskull**](https://github.com/boneskull))
-   [#&#8203;3706](https://github.com/mochajs/mocha/issues/3706): Fix regression wherein `--reporter-option`/`--reporter-options` did not support comma-separated key/value pairs ([**@&#8203;boneskull**](https://github.com/boneskull))

#### 📖 Documentation

-   [#&#8203;3652](https://github.com/mochajs/mocha/issues/3652): Switch from Jekyll to Eleventy ([**@&#8203;Munter**](https://github.com/Munter))

#### 🔩 Other

-   [#&#8203;3677](https://github.com/mochajs/mocha/issues/3677): Add error objects for createUnsupportedError and createInvalidExceptionError ([**@&#8203;boneskull**](https://github.com/boneskull))
-   [#&#8203;3733](https://github.com/mochajs/mocha/issues/3733): Removed unnecessary processing in post-processing hook ([**@&#8203;wanseob**](https://github.com/wanseob))
-   [#&#8203;3730](https://github.com/mochajs/mocha/issues/3730): Update nyc to latest version ([**@&#8203;coreyfarrell**](https://github.com/coreyfarrell))
-   [#&#8203;3648](https://github.com/mochajs/mocha/issues/3648), [#&#8203;3680](https://github.com/mochajs/mocha/issues/3680): Fixes to support latest versions of [unexpected](https://npm.im/unexpected) and [unexpected-sinon](https://npm.im/unexpected-sinon) ([**@&#8203;sunesimonsen**](https://github.com/sunesimonsen))
-   [#&#8203;3638](https://github.com/mochajs/mocha/issues/3638): Add meta tag to site ([**@&#8203;MartijnCuppens**](https://github.com/MartijnCuppens))
-   [#&#8203;3653](https://github.com/mochajs/mocha/issues/3653): Fix parts of test suite failing to run on Windows ([**@&#8203;boneskull**](https://github.com/boneskull))

</details>

---

### Renovate configuration

:date: **Schedule**: "after 9am and before 3pm" (UTC).

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

:recycle: **Rebasing**: Whenever PR is stale, or if you modify the PR title to begin with "`rebase!`".

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

---

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

---

This PR has been generated by [Renovate Bot](https://github.com/marketplace/renovate). View repository job log [here](https://renovatebot.com/dashboard#googleapis/nodejs-asset).

#109 automerged by dpebot
* Release v0.3.0

* Update CHANGELOG.md
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
* updated CHANGELOG.md

* updated package.json

* updated samples/package.json
* updated CHANGELOG.md [ci skip]

* updated package.json [ci skip]

* updated samples/package.json [ci skip]
bcoe and others added 22 commits November 17, 2022 10:53
🤖 I have created a release \*beep\* \*boop\*
---
## [3.20.0](https://www.github.com/googleapis/nodejs-asset/compare/v3.19.0...v3.20.0) (2021-10-19)


### Features

* Update osconfig v1 and v1alpha RecurringSchedule.Frequency with DAILY frequency ([#569](https://www.github.com/googleapis/nodejs-asset/issues/569)) ([af03fd5](https://www.github.com/googleapis/nodejs-asset/commit/af03fd5c4fba4a258acf4c0332991bcb619fa10b))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
* feat!: Update library to use Node 12

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* samples: fix compute test, update dependency to V3
* chore(main): release 4.0.0

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: sofisl <55454395+sofisl@users.noreply.github.com>
🤖 I have created a release *beep* *boop*
---


## [4.1.0](googleapis/nodejs-asset@v4.0.0...v4.1.0) (2022-06-29)


### Features

* support regapic LRO ([#635](googleapis/nodejs-asset#635)) ([140ce9d](googleapis/nodejs-asset@140ce9d))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
* chore(main): release 4.2.0

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* feat: add batchGetEffectiveIamPolicies sample code.

Add batchGetEffectiveIamPolicies sample code and also lint the protobuf
imports.

* chore: fix the Copyright year for getBatchEffectiveIamPolicies.js

* chore: refactor logging and remove loop for checking results

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* chore: modify the logging to print nested Object.

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* chore(main): release 4.3.0

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop*
---


## [4.4.0](googleapis/nodejs-asset@v4.3.0...v4.4.0) (2022-09-22)


### Features

* Add client library support for AssetService v1 SavedQuery APIs ([#662](googleapis/nodejs-asset#662)) ([68c8ece](googleapis/nodejs-asset@68c8ece))


### Bug Fixes

* Preserve default values in x-goog-request-params header ([#658](googleapis/nodejs-asset#658)) ([e351ed1](googleapis/nodejs-asset@e351ed1))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
* chore(main): release 4.5.0

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Copy link
Contributor

@ace-n ace-n left a comment

Choose a reason for hiding this comment

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

Should these files be in asset/snippets/, or the asset directory itself?

@@ -0,0 +1,262 @@
// Copyright 2021 Google LLC
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to include this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This singular test file includes not just the quickstart tests, but tests for all the other samples, as well. They are run sequentially.

I included it in the migration, so that when the language maintainers address the GH issue I created (#2864), they would have the body of the individual tests already and could use their best judgment to modify them as needed to decompose into separate test files. At that point, I imagine the samples.test.js will be deleted.

I figure inclusion of this file here is acceptable tech debt for the migration. Since @sofisl already discarded most of the tests for her migration to google-cloud-node, I didn't want to lose it in the migration/archival process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To address your other comment about migration destination:
i (and I believe most others) are following the example set in the migration guide, written by @grayside. You can see here that the samples end up <product>/snippets:
https://docs.google.com/document/d/1YhpsM5ylBFDunRsd-Vq7pHRNUz3uh9TUbOkJOZwCmOk/edit?resourcekey=0--XOS_x8NmzOS8_Qws-Qsug#bookmark=kix.z1viwa6f4215

Copy link
Contributor

@ace-n ace-n left a comment

Choose a reason for hiding this comment

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

Omitting the snippets directory feels cleaner to me, but IMO it's not worth creating a new PR.

LGTM. ✅

@ahrarmonsur ahrarmonsur merged commit 47773ed into main Nov 18, 2022
@ahrarmonsur ahrarmonsur deleted the nodejs-asset-migration branch November 18, 2022 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.