Skip to content
This repository was archived by the owner on Mar 11, 2026. It is now read-only.

fix: log.write removed option.resource.labels#1219

Merged
losalex merged 2 commits intomainfrom
losalex/fix-1167
Jan 21, 2022
Merged

fix: log.write removed option.resource.labels#1219
losalex merged 2 commits intomainfrom
losalex/fix-1167

Conversation

@losalex
Copy link
Copy Markdown
Contributor

@losalex losalex commented Jan 21, 2022

The snakecaseKeys was always removing labels which were in snake-case

Fixes #1167 🦕

@losalex losalex requested review from a team January 21, 2022 20:07
@product-auto-label product-auto-label Bot added the size: xs Pull request size is extra small. label Jan 21, 2022
@product-auto-label product-auto-label Bot added the api: logging Issues related to the googleapis/nodejs-logging API. label Jan 21, 2022
Copy link
Copy Markdown
Contributor

@minherz minherz left a comment

Choose a reason for hiding this comment

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

lgtm with few questions

Comment thread src/utils/log-common.ts
Comment on lines +60 to +63
const replaced = key.replace(
/[A-Z]/g,
letter => `_${letter.toLowerCase()}`
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not just key.toLowerCase()?

Copy link
Copy Markdown
Contributor Author

@losalex losalex Jan 21, 2022

Choose a reason for hiding this comment

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

you need to add also '_' - I afraid that if I change it now, it might be breaking change...

Comment thread test/utils/log-common.ts
@losalex losalex merged commit 6d7e9ed into main Jan 21, 2022
@losalex losalex deleted the losalex/fix-1167 branch January 21, 2022 21:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: logging Issues related to the googleapis/nodejs-logging API. size: xs Pull request size is extra small.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants