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

fix: rename path template params which is reserved word in strict mode #1124

Merged
merged 7 commits into from
Mar 15, 2022

Conversation

summer-ji-eng
Copy link
Collaborator

  1. nodejs-security-center uses export keyword, preventing compile
  2. nodejs-artifact-registry uses package keyword preventing compile.

TS complains the key words error(see below) when the generator uild a list of resources referenced by this service. We rename the these key words in the strict mode reserved words:

Strict Mode Reserved Words
as
implements
interface
let
package
private
protected
public
static
yield
Identifier expected. 'package' is a reserved word in strict mode. Class definitions are automatically in strict mode.

@@ -20,6 +20,19 @@
* the template engine.
*/

const STRICT_MODE_RESERVED_WORDS = new Set([
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we recently turn on strict mode, or did we just happen to have not bumped into this issue before?

Copy link
Collaborator Author

@summer-ji-eng summer-ji-eng Mar 14, 2022

Choose a reason for hiding this comment

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

As I know, we always use strict mode. The protos are newly added proto.

@snippet-bot
Copy link

snippet-bot bot commented Mar 14, 2022

Here is the summary of possible violations 😱

There is a possible violation for not having product prefix.

The end of the violation section. All the stuff below is FYI purposes only.


Here is the summary of changes.

You are about to add 1 region tag.

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

projectExportPath(project:string,exportParam:string) {
return this.pathTemplates.projectExportPathTemplate.render({
project: project,
exportParam: exportParam,
Copy link
Contributor

@alexander-fenster alexander-fenster Mar 14, 2022

Choose a reason for hiding this comment

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

I believe it should say "export": exportParam here, otherwise it won't get rendered properly. You can do "export" with quotes, gts will remove the quotes if they are not needed.

Copy link
Collaborator Author

@summer-ji-eng summer-ji-eng Mar 15, 2022

Choose a reason for hiding this comment

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

Cool. After couple tries with gax and even created a useless PR in gax😢, finally, I got the idea. I only need to update the identifier in the method parameter. Update the PR and please take another look. Thanks for bearing with me. 🤝

@summer-ji-eng summer-ji-eng merged commit e1ebf3a into main Mar 15, 2022
@summer-ji-eng summer-ji-eng deleted the fix_strict_mode_reserve_words branch March 15, 2022 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants