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

feat: Add destination property into LogEntry#720

Merged
losalex merged 11 commits intomainfrom
losalex/719
Oct 24, 2021
Merged

feat: Add destination property into LogEntry#720
losalex merged 11 commits intomainfrom
losalex/719

Conversation

@losalex
Copy link
Contributor

@losalex losalex commented Oct 21, 2021

Added ResourceName which will provide destination log resource name customization for LogEntry

Fixes #719 ☕️

@losalex
Add ResourceName class which will provide destination log resource na…
5a86394 
…me customization and integrate it with LogEntry
@losalex losalex requested a review from minherz October 21, 2021 02:52
@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/java-logging API. label Oct 21, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 21, 2021
minherz
minherz suggested changes Oct 21, 2021
View reviewed changes
@losalex losalex changed the title Add ResourceName class and integrate it with LogEntry feat: Add ResourceName class and integrate it with LogEntry Oct 22, 2021
minherz
minherz suggested changes Oct 22, 2021
View reviewed changes
Copy link
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.

Please, address a case when the log name destination is lost when parsing it from protobuf.
The code in unit test should be aligned with Java guidelines (constants naming convention and multiple use of the same string literals).
Also consider other non-blocking comments.

@losalex losalex marked this pull request as ready for review October 22, 2021 21:26
@losalex losalex requested review from a team October 22, 2021 21:26
minherz
minherz approved these changes Oct 24, 2021
View reviewed changes
Copy link
Contributor

@minherz minherz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good work! Couple of things to do before you merge:

  • Run formatting again to make sure PR pass lint check
  • Rebase to the latest version of the main branch (ping me if you have concerns about the way to do it)

Than you

@minherz minherz changed the title feat: Add ResourceName class and integrate it with LogEntry feat: Add destination property into LogEntry Oct 24, 2021
@googleapis googleapis deleted a comment from losalex Oct 24, 2021
@losalex losalex merged commit 43ea0b4 into main Oct 24, 2021
@losalex losalex deleted the losalex/719 branch October 24, 2021 20:43
@losalex losalex mentioned this pull request Oct 25, 2021
Closed
@losalex losalex assigned losalex and unassigned simonz130 Feb 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@minherz minherz minherz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@losalex losalex

Labels

api: logging Issues related to the googleapis/java-logging API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@losalex @minherz @simonz130