feature: 180 feature move more scripts to golang #191
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
…pts-to-golang Conflicts: bsctl/go.mod bsctl/go.sum
github-actions
bot
added
size/XXL
github-actions
bot
added
the
size/XXL
Conflicts: bsctl/cmd/version.go bsctl/cmd/version_test.go bsctl/go.mod bsctl/go.sum
Conflicts: bsctl/go.mod
github-actions
bot
added
size/XL
mrlunchbox777
added
changes/source
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances CLI tooling by moving more shell scripts to Go, improving I/O stream handling with tee functionality, integrating schema validation, and enforcing version/CHANGELOG bumps in CI.
- Introduced
ReaderTee/WriterTeewith full unit tests and wired streams through the factory to commands. - Refactored config client to reconcile and validate new
FailValidationAbove10field. - Updated bash workflows and scripts to check version and CHANGELOG consistency and bumped constants.
Reviewed Changes
Copilot reviewed 52 out of 54 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| shared-scripts/k8s/clear-finalizers.sh | New script to bulk-clear Kubernetes namespace finalizers |
| shared-scripts/big-bang/readme-bump.sh | Parameterized Helm docs bump to use a branch variable |
| bsctl/util/k8s/io_streams_tees.go | Added tee implementations for I/O streams |
| bsctl/util/k8s/io_streams_* | Added getters, stores, wrappers, and tests to support new stream API |
| bsctl/util/test/fake_factory.go | Extended fake factory with IOStreamsGetter for CLI testing |
| bsctl/util/config/schemas/* | Introduced FailValidationAbove10 setting with validation |
| bsctl/util/config/client_functions.go | Upgraded getConfig to return errors and perform struct validation |
| bsctl/util/factory.go | Exposed GetStreams() on the factory interface |
| bsctl/static/constants.go | Added ConstantsClient interface and GetDefaultConstants() |
| bsctl/cmd/{completion,version,cmd}.go | Refactored commands to use new streams interface |
| bsctl/go.mod | Bumped Go version and updated indirect dependencies |
| bsctl/scripts/workflows/ | Added scripts for version/CHANGELOG checks and action-validator linting |
| CHANGELOG.md | Bumped project to 0.1.2 and documented new features |
| Makefile | Updated vet target to invoke make vet |
Comments suppressed due to low confidence (3)
bsctl/util/test/fake_factory.go:20
- [nitpick] The field name
streamsInstanceis ambiguous. Consider renaming it tostreamsGetterorioStreamsProviderto clarify it implementsIOStreamsGetter.
streamsInstance: k8s.GetStoreOnlyStreams(),
shared-scripts/k8s/clear-finalizers.sh:1
- There are no tests covering this new cleanup script. Adding integration or unit tests (e.g., via
bats-core) would improve confidence and coverage.
#! /usr/bin/env bash
shared-scripts/big-bang/readme-bump.sh:94
- The updated Helm docs bump script is not covered by tests; consider adding a test or CI step to validate it against a sample chart.
curl -LO https://repo1.dso.mil/big-bang/apps/library-charts/gluon/-/raw/$branch/docs/README.md.gotmpl
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Andrew Shoell <mrlunchbox777@gmail.com>
Merging this branch changes the coverage (1 decrease, 4 increase)
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
Owner
Author
|
looks like there are a few branches in github.com/mrlunchbox777/basic-setup/bsctl/util/config that need to be covered to finish this |
Conflicts: bsctl/go.mod bsctl/go.sum
…mrlunchbox777/basic-setup into 180-feature-move-more-scripts-to-golang
Conflicts: shared-scripts/big-bang/readme-bump.sh
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: #180
Description
Please describe how this PR is addressing the issue and/or why it is being addressed this way.Response: This PR updates pipes and checks for CHANGELOG and version bumps as well as getting non-test coverage up to 100%
Steps 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 coverageshould show 100% on everything except fake factoryRequired 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