Skip to content

fix(retail): add region tag for node.js - Update add fullfillment places #4120

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

Merged
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

'use strict';

// [START retail_add_fulfillment_places]
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The placement of this [START] tag is not ideal for creating a high-quality code sample for a couple of reasons:

  1. require() inside a function: The main function contains require() statements (lines 19-20). This is a Node.js anti-pattern. Imports should be at the top level of a module for readability and to follow convention. By adding the tag here, this anti-pattern is being showcased in an official sample.
  2. 'use strict'; exclusion: The 'use strict'; directive on line 15 is excluded from the sample, making the snippet incomplete.

Recommendation:

To create a more idiomatic and robust code sample, I recommend refactoring the file to move the require() statements to the top level. The [START] tag should then be placed before 'use strict'; to create a complete, self-contained sample.

Example of the ideal structure:

// ... license header ...

// [START retail_add_fulfillment_places]
'use strict';

const {ProductServiceClient} = require('@google-cloud/retail').v2;
const utils = require('../setup/setup-cleanup');

async function main(generatedProductId) {
  // ... function body without require() statements ...
}

// ... rest of the file ...
// [END retail_add_fulfillment_places]

If this refactoring is out of scope, please at least consider moving the [START] tag to line 15, before 'use strict';, to make the sample more complete.

async function main(generatedProductId) {
// Imports the Google Cloud client library.
const {ProductServiceClient} = require('@google-cloud/retail').v2;
Expand Down Expand Up @@ -89,3 +89,4 @@ process.on('unhandledRejection', err => {
});

main(...process.argv.slice(2));
// [END retail_add_fulfillment_places]