-
-
Notifications
You must be signed in to change notification settings - Fork 3
chore: apply code review feedback - improve bash idioms and Go naming conventions #258
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: apply code review feedback - improve bash idioms and Go naming conventions #258
Conversation
…entions Co-authored-by: mrlunchbox777 <9466269+mrlunchbox777@users.noreply.github.com>
mrlunchbox777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
@copilot correct the failing gates |
The CI gate "Code coverage report" is failing because it cannot find a baseline coverage report for the target branch The code coverage workflow ( This is expected behavior when PRs target feature branches instead of main. The "Unit tests" job passes successfully - only the coverage comparison fails due to missing baseline. Options:
My code changes don't cause this issue - it's an infrastructure limitation with the base branch configuration. |
db85c57
into
180-feature-move-more-scripts-to-golang
* bubbling up errors and fixing tests * WIP working on adding io streams * handling nils for the interfaces * adding a check and test * adding some test * simplifying * adding and updating some workflows and adding tests * bumping versions * dropping the extra file * trying to get the workflows to work again * deconflicting the names * adding the action validator * updating the action validator * poking the checks * more fixes * fixing the ignore * trying something else * trying another thing * wildcard * just switching to grep * fixing typo * more debug * finding more stuff * moving back to jobs with steps * bumping and testing more * fixing the constants test * more test fixes * ensuring that major versions are tested, but not worrying about minor or patches * renaming and adding the CHANGELOG bump * testing the logs * tested all of the changelog tested locally and bumping things * test * test * test * test * test * setting back to a real changelog * switching back to a failing changelog until this is ready * attempting the script * adding execute permissions * moving scripts to scripts * forgot a folder * forgot the workflows * add execution * lysdexia * adding test to factory * found WIP * bbctl moved, adjusting, currently broken * still broken, but got main test working * fixing the rest of the tests * docs bump * trying to fix the pipe * trying again * Update shared-scripts/big-bang/readme-bump.sh Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Andrew Shoell <mrlunchbox777@gmail.com> * feat: updating gitalias * feat: finishing this pr so more can be done * fix: update CHANGELOG and version to 0.1.3 for test fixes - Updated CHANGELOG date to 2026-01-16 - Bumped version from 0.1.2 to 0.1.3 - Documented test expectation updates after bbctl v1.5.0 upgrade * chore: apply code review feedback - improve bash idioms and Go naming conventions (#258) * Initial plan * chore: apply review comments - improve bash idioms and Go naming conventions Co-authored-by: mrlunchbox777 <9466269+mrlunchbox777@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mrlunchbox777 <9466269+mrlunchbox777@users.noreply.github.com> --------- Signed-off-by: Andrew Shoell <mrlunchbox777@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: mrlunchbox777 <9466269+mrlunchbox777@users.noreply.github.com>
Thanks for contributing!
Background
To pass the semanic-prs check, ensure you prefix your title with one of the
.typesin semanic.yml, followed by a:, e.g.feature: My PRIssue
What issue are you resolving with this PR? Please provide the number or link. _NOTE:_ If you don't have an issue for this work, please create one before creating this PR.Response: Addresses review comments from PR #191 (review 3668887529)
Description
Please describe how this PR is addressing the issue and/or why it is being addressed this way.Response: Applies code style improvements from automated review:
== ""with-zand! -zwith-nfor idiomatic empty/non-empty checkspFlagtopflagthroughoutmain.goto follow Go's lowercase package alias conventionFiles modified:
bsctl/scripts/workflows/docs-bump_docs-bump_CHANGELOG-bump.sh- empty string checkshared-scripts/k8s/clear-finalizers.sh- two non-empty string checksbsctl/main.go- import alias and all usagesSteps to Reproduce and Test
Please give us a step-by-step guide to reproduce the bug. A link to the steps in the issue is enough.Response:
make build- verifies Go changes compileRequired Checkboxes
All of these checkboxes are required for PR's to be considered.
PR Checks
Code of Conduct
By submitting this PR, you agree to follow our Code of Conduct
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.