SREP-1107: Added a new 'managed-scripts' subcommand to osdctl promote#855
Draft
Nikokolas3270 wants to merge 1 commit intoopenshift:masterfrom
Draft
SREP-1107: Added a new 'managed-scripts' subcommand to osdctl promote#855Nikokolas3270 wants to merge 1 commit intoopenshift:masterfrom
osdctl promote#855Nikokolas3270 wants to merge 1 commit intoopenshift:masterfrom
Conversation
This also contains the following changes: - 'pko' promote subcommand removed as it was no longer applying to any saas file - '--osd' and '--hcp' options removed as a there is no service which could be both hcp and osd - Now exclusively using 'sigs.k8s.io/kustomize/kyaml' as both a YAML parser & marshaller as 'gopkg.in/yaml.v3' is no longer supported - Now interacting with Git repo with 'github.com/go-git/go-git/v6' lib rather than running shell commands - Code has been factorized
openshift-ci-robot
added
the
jira/valid-reference
|
@Nikokolas3270: This pull request references SREP-1107 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThe promote command is refactored to replace git-based app-interface operations with a registry-driven service discovery approach. The PKO subcommand is removed and replaced with ManagedScripts. New utility packages provide callback-based service promotion workflows via YAML-based configurations. Related Dynatrace, SaaS, and path utilities are updated or removed accordingly. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Nikokolas3270 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
@Nikokolas3270: This pull request references SREP-1107 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
cmd/promote/dynatrace/dt_utils.go (2)
175-178:⚠️ Potential issue | 🟠 MajorReturn immediately when opening the file fails.
The current flow logs the error and continues, which can pass an invalid file handle into downstream processing.
Proposed fix
file, err := Open(filename) if err != nil { - fmt.Println(err) + return fmt.Errorf("failed to open file '%s': %w", filename, err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/promote/dynatrace/dt_utils.go` around lines 175 - 178, The Open(filename) call currently prints the error and continues, which can pass an invalid file handle into downstream code; update the error handling at the Open(...) site so that on err you return the error (or otherwise abort the function) instead of just fmt.Println(err). Locate the Open(filename) invocation in dt_utils.go and modify the surrounding function to propagate the error (or call return) immediately when err != nil, ensuring downstream code does not receive a nil/invalid file handle.
142-153:⚠️ Potential issue | 🟠 MajorPropagate
os.ReadDirfailures instead of discarding them.Read errors are currently ignored at multiple levels, so the promotion can incorrectly continue as if files were scanned successfully.
Proposed fix
- items, _ := os.ReadDir(dir) + items, err := os.ReadDir(dir) + if err != nil { + return "", fmt.Errorf("failed to read directory '%s': %w", dir, err) + } ... - subitems, _ := os.ReadDir(subDir) + subitems, err := os.ReadDir(subDir) + if err != nil { + return "", fmt.Errorf("failed to read directory '%s': %w", subDir, err) + } ... - subitems2, _ := os.ReadDir(subDir2) + subitems2, err := os.ReadDir(subDir2) + if err != nil { + return "", fmt.Errorf("failed to read directory '%s': %w", subDir2, err) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/promote/dynatrace/dt_utils.go` around lines 142 - 153, The os.ReadDir calls in the nested directory walk (the reads that populate items, subitems and subitems2) currently ignore their returned errors; change each call to capture the error (e.g., items, err := os.ReadDir(dir)) and propagate it instead of discarding it—return the error (or wrap and return) from the enclosing function (the directory-walking function in dt_utils.go) so failures stop the promotion; do the same for subitems and subitems2 reads and adjust calling code to handle the returned error.cmd/promote/dynatrace/dynatrace.go (1)
73-74:⚠️ Potential issue | 🟠 MajorDon’t ignore list errors before successful exit.
Both list branches discard the returned error and then exit with success, which can hide real failures from users and automation.
Proposed fix
- _ = listDynatraceModuleNames(dynatraceConfig) - os.Exit(0) + if err := listDynatraceModuleNames(dynatraceConfig); err != nil { + return fmt.Errorf("failed to list dynatrace modules: %w", err) + } + return nil ... - _ = listServiceIds(servicesRegistry) - os.Exit(0) + if err := listServiceIds(servicesRegistry); err != nil { + return fmt.Errorf("failed to list dynatrace components: %w", err) + } + return nilAlso applies to: 117-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/promote/dynatrace/dynatrace.go` around lines 73 - 74, The calls to listDynatraceModuleNames currently discard the returned error and then call os.Exit(0); change both call sites to capture the error (e.g., err := listDynatraceModuleNames(dynatraceConfig)), check if err != nil, print or log the error to stderr (or use the existing logger) and call os.Exit(1); if no error, then exit with success. This ensures failures from listDynatraceModuleNames are surfaced instead of being silently ignored.
🧹 Nitpick comments (5)
cmd/promote/dynatrace/dt_utils.go (1)
108-124: Reset module caches inGetModulesNamesbefore repopulating.
ModulesSliceandModulesFilesMapare package globals and currently accumulate across repeated calls.Proposed fix
func GetModulesNames(baseDir, dir string) ([]string, error) { + ModulesSlice = ModulesSlice[:0] + ModulesFilesMap = map[string]string{} + dirGlob := filepath.Join(baseDir, dir) filepaths, err := os.ReadDir(dirGlob)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/promote/dynatrace/dt_utils.go` around lines 108 - 124, GetModulesNames currently appends to package globals ModulesSlice and ModulesFilesMap on each call; modify GetModulesNames to reset/clear ModulesSlice and reinitialize or clear ModulesFilesMap at the start of the function (before reading dir entries) so repeated calls don't accumulate stale data; locate the function GetModulesNames and add logic to set ModulesSlice = nil (or empty slice) and ModulesFilesMap = make(map[string]string) (or clear existing entries) before populating them.cmd/promote/utils/service.go (4)
42-44: Inconsistent receiver name onyamlDoc.
GetFilePath(line 38) andSave(line 46) used, butGetNameusess. Use a consistent receiver name across all methods of the same type.Proposed fix
-func (s *yamlDoc) GetName() string { - return s.name +func (d *yamlDoc) GetName() string { + return d.name }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/promote/utils/service.go` around lines 42 - 44, The GetName method for type yamlDoc uses an inconsistent receiver name `s`; change its receiver to `d` to match the other methods on yamlDoc (e.g., GetFilePath and Save) so all methods use the same receiver identifier; update the method signature for GetName to use `d` and keep the body returning d.name.
413-419: Nit: unconventional blank line in error handling.The blank line between the call on line 414 and the
if err != nilon line 416 breaks the standard Go error-handling idiom. Consider removing it for consistency.Proposed fix
if newHash == "" { newHash, err = repo.GetHeadHash() - if err != nil { return err } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/promote/utils/service.go` around lines 413 - 419, Remove the extra blank line in the error handling after calling repo.GetHeadHash so the idiomatic Go pattern is preserved; specifically, in the block that assigns newHash (the call to repo.GetHeadHash) and the subsequent if err != nil block, bring the if directly under the call (no blank line) to keep the standard newHash, err := repo.GetHeadHash / if err != nil style used elsewhere.
296-330: Partial failure leaves committed state in the local branch.Each
resourceTemplatePromotion.promotecall saves the YAML and commits independently (lines 307, 318). If a later promotion in the loop (line 428) fails, earlier promotions are already committed to the local branch. This is likely acceptable since the user must manually push, but consider documenting this behavior or logging a warning when a partial failure occurs so users know what's been committed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/promote/utils/service.go` around lines 296 - 330, The promote function (resourceTemplatePromotion.promote) commits each resource independently (uses callbacks.SetTargetHash, service.Save, service.appInterfaceClone.Commit) which can leave the local branch partially committed if a later promotion fails; update the caller that loops over promotions (the code that invokes resourceTemplatePromotion.promote) to track which promotions succeeded and, on any subsequent error, emit a clear warning/log that lists the committed resource relPaths and informs the user that the local branch contains partial commits requiring manual push/revert; alternatively expose a boolean or error type from promote indicating "committed" so the caller can build that list before logging the warning.
21-21: Export theyamlDoctype to match its exported function.
ReadYamlDocFromFileis exported and used externally (e.g.,cmd/promote/saas/saas.go:118), but it returns*yamlDoc, which is unexported. This violates Go conventions — external callers cannot declare variables of this type. ExportyamlDocasYamlDoc(or use the lowercase-exported pattern if intentional, but document it).Additionally, the receiver names for
yamlDocmethods are inconsistent:GetFilePath()andSave()use receiverd, whileGetName()(line 42) uses receivers. Standardize on a single receiver name liked.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/promote/utils/service.go` at line 21, The exported function ReadYamlDocFromFile returns an unexported type yamlDoc which prevents external callers from using its value; rename the type to exported YamlDoc (or change the function to return an exported interface) and update all references accordingly (including ReadYamlDocFromFile's return type and any uses in cmd/promote/saas/saas.go). Also standardize method receiver names on that type: change the GetName receiver from 's' to 'd' to match GetFilePath and Save so all methods on yamlDoc/YamlDoc use a consistent receiver (d). Ensure you update any imports/usages and tests to the new exported type name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/promote/managedscripts/managed_scripts.go`:
- Around line 30-32: The FilterTargets method currently hardcodes
prodNamespaceRef when calling utils.FilterTargetsContainingNamespaceRef, so the
configured --namespaceRef value is ignored; update
promoteCallbacks.FilterTargets to pass the instance/configured namespace
reference (e.g., c.namespaceRef or the field that holds the CLI flag) into
utils.FilterTargetsContainingNamespaceRef instead of prodNamespaceRef, ensuring
the same change is applied where else prodNamespaceRef is used for target
selection so the runtime flag actually influences filtering.
In `@cmd/promote/saas/saas.go`:
- Line 234: Update the CLI help/example string to use the canonical flag
--serviceId instead of the legacy alias --serviceName: locate the usage text
that currently reads `osdctl promote saas --serviceName <service> --gitHash
<git-hash>` (in the promote command's help/usage output) and replace
`--serviceName` with `--serviceId`; ensure any adjacent help text or examples
referencing the legacy alias are also updated so the example consistently shows
`--serviceId <service>` with `--gitHash <git-hash>`.
- Around line 271-274: Add a fail-fast required check for ops.serviceId before
calling servicesRegistry.GetService to give a clearer error for promote mode:
validate that ops.serviceId is non-empty (e.g., at the start of the promote
handler or before the GetService call), and return a descriptive error if it's
empty (mentioning that --serviceId is required in promote mode). Update the code
path where servicesRegistry.GetService(ops.serviceId) is invoked (reference
ops.serviceId and the promote handler function) to perform this explicit check
and only call GetService when the value is present.
- Around line 38-43: The current logic returns the original filePath when
deploy.yaml is missing, which can register a directory as a service; change the
code to only return valid regular file paths: after checking subFilePath, also
verify that filePath itself is a regular file (use os.Stat(filePath) and
fileInfo.Mode().IsRegular()) before returning it, and if it's a directory or not
a regular file return an empty string (or an error sentinel) so the caller can
skip this SaaS entry; adjust callers of this helper to treat empty string as
"skip invalid entry".
In `@cmd/promote/utils/git_repo.go`:
- Around line 54-89: The code pre-marks commonAncestorCommit.Hash in
visitedHashes before traversal which allows walks that never actually reach
commonAncestorHash to succeed silently; modify the traversal in the function
containing queue, visitedHashes, commit.IsAncestor and the final sb.WriteString
branch so that you either do not pre-populate visitedHashes with
commonAncestorCommit.Hash or (preferably) add a post-traversal check that
verifies visitedHashes contains commonAncestorHash (or that you visited it
during traversal) and return an explicit error if it was not reached; use the
symbols visitedHashes, commonAncestorCommit / commonAncestorHash, queue,
commit.IsAncestor and the final return path to locate where to add the check.
In `@cmd/promote/utils/service.go`:
- Around line 212-229: The function FilterTargetsContainingNamespaceRef
currently uses strings.Contains(visitedNamespaceRef, namespaceRef) which allows
substring matches; change the check to a precise match (e.g., verify the path
ends with the expected segment or exact identifier) by replacing the contains
logic with a more strict comparison such as using strings.HasSuffix or parsing
visitedNamespaceRef into path segments and comparing the final segment to
namespaceRef (operate on visitedNamespaceRef and namespaceRef variables inside
FilterTargetsContainingNamespaceRef so only exact/segment matches are
considered).
- Around line 141-146: The code reads appRelFilePath from
yamlDoc.rootNode.GetString and directly uses
filepath.Join(appInterfaceClone.GetPath(), "data", appRelFilePath) before
calling readApplicationFromFile; sanitize appRelFilePath by running
filepath.Clean, reject absolute paths or any path that resolves outside the
intended base (e.g. contains ".." segments that escape the
appInterfaceClone.GetPath()/data directory), and construct the final path by
joining the cleaned relative path to appInterfaceClone.GetPath()/data then
verifying the resulting path has the base directory as a prefix before calling
readApplicationFromFile.
- Around line 389-394: Iterate deterministically over oldHashToTargetNodes by
collecting its keys into a slice, sorting that slice (using sort.Strings), and
then building resourceTemplatePromotions in the sorted order (instead of ranging
the map directly); update the import block to include "sort". Ensure the same
symbols are used: oldHashToTargetNodes, resourceTemplatePromotions,
resourceTemplatePromotion, and that downstream code that calls promote will now
see a stable commit order.
In `@cmd/promote/utils/services_registry.go`:
- Around line 19-33: The constructor NewServicesRegistry should guard against
nil inputs to avoid panics: at the start of the function check if
appInterfaceClone is nil and if so return a descriptive error (do not call
appInterfaceClone.GetPath()); also check if validateServiceFilePathCallback is
nil and return a descriptive error before invoking it later; update
callers/tests if necessary to pass non-nil values or to handle the returned
error from NewServicesRegistry.
- Around line 36-37: The code silently overwrites existing entries in the
serviceIdToFilePath map (variable serviceId) when scanning files; change the
insertion logic to fail fast on duplicates by checking if serviceId already
exists in serviceIdToFilePath and returning/propagating an error (or logging and
exiting) that includes the conflicting serviceId and both file paths (existing
value and serviceFilePath) so callers of the scanning function can handle the
failure instead of allowing silent overwrite.
In `@docs/osdctl_promote_managedscripts.md`:
- Around line 5-39: Add language identifiers to all fenced code blocks in
docs/osdctl_promote_managedscripts.md to satisfy MD040: mark the command usage
block as "text" (or "bash" if preferred), the example block as "bash", and both
options blocks (Options and Options inherited from parent commands) as "text".
Update the fences surrounding the snippets that contain "osdctl promote
managedscripts [flags]", the example starting with "# Promote managed-scripts
repo", the options list beginning with "--appInterfaceDir", and the inherited
options list beginning with "--as" to include the appropriate language tags.
In `@docs/osdctl_promote_saas.md`:
- Around line 22-30: The fenced code block showing CLI flags in the saas
promotion docs lacks a language identifier which triggers markdownlint MD040;
update the triple-backtick opening fence to include a language (e.g., ```text)
for the block containing the lines starting with "--appInterfaceDir" and ending
with "--serviceId" so the block is treated as plain text and the linter warning
is resolved.
- Line 17: The example for the CLI invocation uses the deprecated alias flag
--serviceName; update the example for the command osdctl promote saas to use the
canonical flag --serviceId instead (replace the --serviceName token with
--serviceId in the example line) so the documentation shows the current,
supported flag.
In `@docs/README.md`:
- Around line 3938-3940: Add a language identifier to the fenced code block that
contains the line "osdctl promote managedscripts [flags]" so markdownlint MD040
is satisfied; modify the opening fence from ``` to ```text (or ```bash) around
the block in README.md so it becomes ```text followed by the command and closing
``` to preserve formatting.
---
Outside diff comments:
In `@cmd/promote/dynatrace/dt_utils.go`:
- Around line 175-178: The Open(filename) call currently prints the error and
continues, which can pass an invalid file handle into downstream code; update
the error handling at the Open(...) site so that on err you return the error (or
otherwise abort the function) instead of just fmt.Println(err). Locate the
Open(filename) invocation in dt_utils.go and modify the surrounding function to
propagate the error (or call return) immediately when err != nil, ensuring
downstream code does not receive a nil/invalid file handle.
- Around line 142-153: The os.ReadDir calls in the nested directory walk (the
reads that populate items, subitems and subitems2) currently ignore their
returned errors; change each call to capture the error (e.g., items, err :=
os.ReadDir(dir)) and propagate it instead of discarding it—return the error (or
wrap and return) from the enclosing function (the directory-walking function in
dt_utils.go) so failures stop the promotion; do the same for subitems and
subitems2 reads and adjust calling code to handle the returned error.
In `@cmd/promote/dynatrace/dynatrace.go`:
- Around line 73-74: The calls to listDynatraceModuleNames currently discard the
returned error and then call os.Exit(0); change both call sites to capture the
error (e.g., err := listDynatraceModuleNames(dynatraceConfig)), check if err !=
nil, print or log the error to stderr (or use the existing logger) and call
os.Exit(1); if no error, then exit with success. This ensures failures from
listDynatraceModuleNames are surfaced instead of being silently ignored.
---
Nitpick comments:
In `@cmd/promote/dynatrace/dt_utils.go`:
- Around line 108-124: GetModulesNames currently appends to package globals
ModulesSlice and ModulesFilesMap on each call; modify GetModulesNames to
reset/clear ModulesSlice and reinitialize or clear ModulesFilesMap at the start
of the function (before reading dir entries) so repeated calls don't accumulate
stale data; locate the function GetModulesNames and add logic to set
ModulesSlice = nil (or empty slice) and ModulesFilesMap =
make(map[string]string) (or clear existing entries) before populating them.
In `@cmd/promote/utils/service.go`:
- Around line 42-44: The GetName method for type yamlDoc uses an inconsistent
receiver name `s`; change its receiver to `d` to match the other methods on
yamlDoc (e.g., GetFilePath and Save) so all methods use the same receiver
identifier; update the method signature for GetName to use `d` and keep the body
returning d.name.
- Around line 413-419: Remove the extra blank line in the error handling after
calling repo.GetHeadHash so the idiomatic Go pattern is preserved; specifically,
in the block that assigns newHash (the call to repo.GetHeadHash) and the
subsequent if err != nil block, bring the if directly under the call (no blank
line) to keep the standard newHash, err := repo.GetHeadHash / if err != nil
style used elsewhere.
- Around line 296-330: The promote function (resourceTemplatePromotion.promote)
commits each resource independently (uses callbacks.SetTargetHash, service.Save,
service.appInterfaceClone.Commit) which can leave the local branch partially
committed if a later promotion fails; update the caller that loops over
promotions (the code that invokes resourceTemplatePromotion.promote) to track
which promotions succeeded and, on any subsequent error, emit a clear
warning/log that lists the committed resource relPaths and informs the user that
the local branch contains partial commits requiring manual push/revert;
alternatively expose a boolean or error type from promote indicating "committed"
so the caller can build that list before logging the warning.
- Line 21: The exported function ReadYamlDocFromFile returns an unexported type
yamlDoc which prevents external callers from using its value; rename the type to
exported YamlDoc (or change the function to return an exported interface) and
update all references accordingly (including ReadYamlDocFromFile's return type
and any uses in cmd/promote/saas/saas.go). Also standardize method receiver
names on that type: change the GetName receiver from 's' to 'd' to match
GetFilePath and Save so all methods on yamlDoc/YamlDoc use a consistent receiver
(d). Ensure you update any imports/usages and tests to the new exported type
name.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (23)
cmd/promote/cmd.gocmd/promote/dynatrace/dt_utils.gocmd/promote/dynatrace/dynatrace.gocmd/promote/dynatrace/utils_test.gocmd/promote/git/app_interface.gocmd/promote/git/app_interface_test.gocmd/promote/git/service_repo.gocmd/promote/git/service_repo_test.gocmd/promote/managedscripts/managed_scripts.gocmd/promote/pathutil/pathutil_test.gocmd/promote/pko/pko.gocmd/promote/saas/saas.gocmd/promote/saas/utils.gocmd/promote/saas/utils_test.gocmd/promote/utils/app_interface_clone.gocmd/promote/utils/git_repo.gocmd/promote/utils/service.gocmd/promote/utils/services_registry.godocs/README.mddocs/osdctl_promote.mddocs/osdctl_promote_managedscripts.mddocs/osdctl_promote_saas.mdgo.mod
💤 Files with no reviewable changes (8)
- cmd/promote/pathutil/pathutil_test.go
- cmd/promote/pko/pko.go
- cmd/promote/saas/utils.go
- cmd/promote/git/app_interface_test.go
- cmd/promote/git/service_repo.go
- cmd/promote/git/app_interface.go
- cmd/promote/git/service_repo_test.go
- cmd/promote/saas/utils_test.go
MateSaary
reviewed
Feb 25, 2026
Member
There was a problem hiding this comment.
The promotion command's code finally looks much better 😄 Added a few comments, aside from that;
-
There was decent test coverage for the command before which is now gone; would be nice to have it back but can also be done as a follow-up, we do already have a card for this in backlog: https://issues.redhat.com/browse/SREP-1278
-
it looks like pathutil/pathutil.go isn't being used anywhere now, so we could remove that orphaned code with this PR too
I tried to go through the coderabbit suggestions as well and commented/reacted to them based on whether or not they make sense 😄
| func (a *AppInterfaceClone) Commit(commitMessage string) error { | ||
| addPattern := "." | ||
| if err := a.workTree.AddGlob(addPattern); err != nil { | ||
| return fmt.Errorf("failed to adds file to the git index : %v", err) |
Member
There was a problem hiding this comment.
Suggested change
| return fmt.Errorf("failed to adds file to the git index : %v", err) | |
| return fmt.Errorf("failed to add files to the git index : %v", err) |
| osdctl promote dynatrace --terraform --module=<module-name>`, | ||
|
|
||
| Run: func(cmd *cobra.Command, args []string) { | ||
| RunE: func(cmd *cobra.Command, args []string) error { |
Member
There was a problem hiding this comment.
This predates this refactor, but the os.Exit(1) calls here should probably be replaced with typical error handling otherwise the RunE won't error handle properly anyways
Contributor
Author
There was a problem hiding this comment.
Thanks for your comment. I was hesitating between the 2 solutions (use RunE and return errors VS use Run and os.Exit(1)) and yeah I didn't finish to migrate back to the former solution.
| } else { | ||
| fmt.Printf("%-*s (no specific path)\n", maxLen, service) | ||
| } | ||
| func validateDynatraceServiceFilePath(filePath string) string { |
Member
There was a problem hiding this comment.
ServicesSlice and ServicesFilesMap are defined a few lines up, but they're not used anywhere anymore so it's safe to get rid of them 🙂
Contributor
Author
Thanks :)
True, I am still working on the unit-tests, no worries. I just wanted to have people a first look on that finally quite big change.
Thanks! I will TAL as well |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This also contains the following changes:
Summary by CodeRabbit
Release Notes
New Features
osdctl promote managedscriptscommand for promoting managed-scripts repository commitsImprovements
Removed
osdctl promote packagecommand