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(gha): output headers cmd to stdout, not stderr #80

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

CalebAlbers
Copy link
Member

🛠️ Description

Previously, the GitHub Actions ::startgroup:: and ::endgroup directives were being sent to stdout, but all command output from Copywrite was being sent to stderr. This caused an issue that was especially common for the copywrite headers --plan command, where the list of files missing headers would be partially included and partially excluded from the logging group.

As a fix I set Cobra up to print to stdout by default, and updated the logger we send to the addlicense module to also use stdout. Ideally, we'd restructure logging entirely within the module, but I'm leaving that for a future PR.

🔗 External Links

#79

👍 Definition of Done

  • New functionality works?

🤔 Can be merged upon approval?

Previously, the GitHub Actions `::startgroup::` and `::endgroup` directives were being sent to stdout, but all command output from Copywrite was being sent to stderr. This caused an issue that was especially common for the `copywrite headers --plan` command, where the list of files missing headers would be partially included and partially excluded from the logging group.

As a fix I set Cobra up to print to stdout by default, and updated the logger we send to the addlicense module to also use stdout. Ideally, we'd restructure logging entirely within the module, but I'm leaving that for a future PR.
@CalebAlbers
Copy link
Member Author

@dlaguerta do you happen to know what's going on with the failing build? Looks like it might be affecting everything due to a bump in goreleaser?

@CalebAlbers CalebAlbers merged commit e76cf19 into main Aug 8, 2023
@CalebAlbers CalebAlbers deleted the fix-gha-race-condition branch August 8, 2023 15:56
CalebAlbers added a commit that referenced this pull request Nov 7, 2023
Previously, the `copywrite headers` command would output a lot of `[DEBUG] skipping file...` statements, esp. when large folders were excluded (looking at you, `NODE_MODULES`). This made for a poor experience, and was a bug introduced when I made #80 - as in an attempt to fix an issue with GHA log group directives and addlicense sending to stdout and stderr, respectively, I introduced a bug that ignored the `COPYWRITE_LOG_LEVEL` env var.

This PR fixes that error while ensuring the GHA log grouping still works by redirecting the `cliLogger` we use throughout the headers cmd (and others) to stdout by default. This doesn't appear to be a large shift or change for other commands, but does make a big difference for the headers command.
CalebAlbers added a commit that referenced this pull request Nov 13, 2023
Previously, the `copywrite headers` command would output a lot of `[DEBUG] skipping file...` statements, esp. when large folders were excluded (looking at you, `NODE_MODULES`). This made for a poor experience, and was a bug introduced when I made #80 - as in an attempt to fix an issue with GHA log group directives and addlicense sending to stdout and stderr, respectively, I introduced a bug that ignored the `COPYWRITE_LOG_LEVEL` env var.

This PR fixes that error while ensuring the GHA log grouping still works by redirecting the `cliLogger` we use throughout the headers cmd (and others) to stdout by default. This doesn't appear to be a large shift or change for other commands, but does make a big difference for the headers command.
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