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

Improve log messages to use the singular or plural forms instead of "noun(s)" #37017

Closed
wants to merge 1 commit into from

Conversation

asashour
Copy link
Contributor

This changes the log to use the singular form or the plural one based on the number of endpoints.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 17, 2023
@snicoll
Copy link
Member

snicoll commented Aug 17, 2023

@asashour thanks. This is the second PR of that nature from you (#36103). I think it would be better to consider the entire code base and do this consistently. Also, please take the review of the previous PRs into account.

Would you amend your PR to replace those usages consistently?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Aug 17, 2023
@jonatan-ivanov
Copy link
Member

I've missed #36103 but if I can share my two cents:
I think this is definitely better for humans to read but if someone wants to process these log entries and if the log messages vary based on some condition on the producer side (endpoint/endpoints), they might need to "reimplement" that logic on the consumer side. E.g.: let's say you want to match these lines with regex, after this change you need to write a slightly more complicated one.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 17, 2023
@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Aug 18, 2023
@asashour asashour changed the title Change EndpointLinksResolver log to use "endpoint" or "endpoints", not "endpoint(s)" Change log messages to use the singular or plural forms instead of "nouns(s)" Aug 18, 2023
@asashour
Copy link
Contributor Author

Thanks for feedback. I hope all cases are covered, as checkout on Windows fails, and a manual search/edit on Ubuntu was done.

It is true that parsing the log messages would be a little more difficult, but I think the use case of logs are meant more for humans, and having a better language overweight the effort for the less likely message processing.

@asashour asashour changed the title Change log messages to use the singular or plural forms instead of "nouns(s)" Change log messages to use the singular or plural forms instead of "noun(s)" Aug 18, 2023
@philwebb philwebb added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Sep 27, 2023
@philwebb philwebb added this to the 3.x milestone Sep 27, 2023
@wilkinsona
Copy link
Member

We discussed this a few months ago and, just to capture our discussion while I can still remember it, I wanted to note that we did not agree that this change would make the log messages harder to parse. With these changes, the singular or plural will tell you if there's one or many values to come. Previously that was unknown and any parsing had to cope with both.

@wilkinsona
Copy link
Member

Thanks very much, @asashour.

@wilkinsona wilkinsona closed this Jan 11, 2024
@wilkinsona wilkinsona modified the milestones: 3.3.x, 3.3.0-M1 Jan 11, 2024
@wilkinsona wilkinsona changed the title Change log messages to use the singular or plural forms instead of "noun(s)" Improve log messages to use the singular or plural forms instead of "noun(s)" Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants