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: Move bom to top level subcmd, simplify shouldSkipInternalUserns #489

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

smoser
Copy link
Contributor

@smoser smoser commented Aug 22, 2023

The bom commands are useful outside of stacker, and since they're exposed to the user, they should not be part of the hidden internal-go interface.

So, change:

- stacker internal-go bom-discover
+ stacker bom discover

Also, shouldSkipInternalUserns was overly complicated as a result of iterative development. Its simple now and based only on the subcommand name.

What type of PR is this?

Which issue does this PR fix:

What does this PR do / Why do we need it:

If an issue # is not available please add repro steps and logs showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades?

Does this PR introduce any user-facing change?:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@smoser smoser requested review from rchincha and hallyn as code owners August 22, 2023 21:15
@smoser smoser force-pushed the fix/bom-not-internal branch from f496e3e to b81c240 Compare August 22, 2023 21:17
@smoser
Copy link
Contributor Author

smoser commented Aug 22, 2023

The majority of the code change here is just code-motion.
code moved from cmd/stacker/internal_go.go to cmd/stacker/bom.go.

@smoser smoser force-pushed the fix/bom-not-internal branch 2 times, most recently from d5f3b77 to 659e8d6 Compare August 23, 2023 01:47
@smoser smoser mentioned this pull request Aug 23, 2023
@smoser smoser force-pushed the fix/bom-not-internal branch from 659e8d6 to 0ed8726 Compare August 23, 2023 11:55
The bom commands are useful outside of stacker, and since
they're exposed to the user, they should *not* be part of the
hidden internal-go interface.

So, change:

 - stacker internal-go bom-discover
 + stacker bom discover

Also, shouldSkipInternalUserns was overly complicated as a result
of iterative development.  It is simpler now and based only
on the subcommand name with a single special case for
testsuite-check-overlay.

Signed-off-by: Scott Moser <smoser@brickies.net>
@smoser smoser force-pushed the fix/bom-not-internal branch from 0ed8726 to 540b3a1 Compare August 23, 2023 14:18
@smoser smoser merged commit 2b35592 into project-stacker:main Aug 23, 2023
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.

3 participants