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

chore: Clarify logs in determining merged project configs #5255

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lukemassa
Copy link
Contributor

what

Collapse and clarify logs in determining merged project configs.

why

I didn't quite understand this when going through #5245, but the logs and comments under automatic detection made it seem like it was conditional on other projects not being found, however that's not the case. If automatic detection is enabled, we simply run it. Maybe that used to be the case, but now this log and comment is a bit misleading and redundant.

This also allowed fewer parameters to be passed into this function, which feels more correct to me; it's a detail that this function doesn't quite "know" that the reason hasRepoCfg is false is that it couldn't find the file. That logging can and should happen closer to when the parsing actually happens.

tests

Normal CI

references

@lukemassa lukemassa requested review from a team as code owners January 19, 2025 21:25
@lukemassa lukemassa requested review from chenrui333, nitrocode and X-Guardian and removed request for a team January 19, 2025 21:25
@github-actions github-actions bot added the go Pull requests that update Go code label Jan 19, 2025
@dosubot dosubot bot added the refactoring Code refactoring that doesn't add additional functionality label Jan 19, 2025
Copy link
Contributor

@jamengual jamengual left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code lgtm This PR has been approved by a maintainer refactoring Code refactoring that doesn't add additional functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants