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: added missing spaces #2386

Merged
merged 1 commit into from
Sep 19, 2024
Merged

fix: added missing spaces #2386

merged 1 commit into from
Sep 19, 2024

Conversation

Absobel
Copy link
Contributor

@Absobel Absobel commented Mar 6, 2024

image

There were missing spaces there, so I added them to make it pretty.

@Absobel Absobel requested a review from Jguer as a code owner March 6, 2024 15:56
@Jguer
Copy link
Owner

Jguer commented Mar 7, 2024

Hey @Absobel is it possible to do this formatting where the errors are output instead?

@Absobel
Copy link
Contributor Author

Absobel commented Mar 27, 2024

I forgot I made this, I just had the problem again.
Anyway, would it be better to modify al the calls to Warnln following a call to DependOn ? (it does that only for circular dependencies tho)

if err := graph.DependOn(depName, parentPkgName); err != nil {
g.logger.Warnln(depString, " ", parentPkgName, " ", err)
}

But its strange because the calls to Errorln or things like that don't seem to have this problem. And I don't know how to reliably test that because I would need to find a packages with circular dependencies or smth

Don't hesitate to tell me if I'm missing something totally obvious, I'm not used to make contributions

Copy link
Owner

@Jguer Jguer left a comment

Choose a reason for hiding this comment

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

Let's merge this one for now

@Jguer Jguer enabled auto-merge (squash) May 2, 2024 10:55
@Jguer Jguer disabled auto-merge September 19, 2024 12:15
@Jguer Jguer merged commit bea53a4 into Jguer:next Sep 19, 2024
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.

2 participants