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

Evidence CLI - update fields names #18

Merged
merged 13 commits into from
Aug 6, 2024
Merged

Evidence CLI - update fields names #18

merged 13 commits into from
Aug 6, 2024

Conversation

osaidwtd
Copy link
Contributor

@osaidwtd osaidwtd commented Aug 4, 2024

No description provided.

@lesnerd lesnerd self-requested a review August 5, 2024 07:40
@lesnerd lesnerd self-assigned this Aug 5, 2024
@lesnerd lesnerd added the feature request New feature or request label Aug 5, 2024
@osaidwtd osaidwtd merged commit 992164e into jfrog:main Aug 6, 2024
6 checks passed
var execFunc = func(command commands.Command) error {
return commands.Exec(command)
}
var execFunc = commands.Exec

func createEvidence(c *components.Context) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c -> ctx (as agreed)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


func (erc *evidenceReleaseBundleCommand) validateEvidenceReleaseBundleContext(ctx *components.Context) error {
if !ctx.IsFlagSet(releaseBundleVersion) || assertValueProvided(ctx, releaseBundleVersion) != nil {
return errorutils.CheckErrorf("'releaseBundleVersion' is a mandatory field for creating a Release Bundle evidence: --%s", releaseBundleVersion)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we explicitly say Release Bundle v2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

evidence working with RBV2 only, RBV1 is something going to be sunset for customers.

}

func validateCreateEvidenceContext(c *components.Context) error {
func validateCreateEvidenceCommonContext(c *components.Context) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here c -> ctx

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple places in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

repoPath = "repo-path"
key = "key"
keyId = "key-name"
evidencePrefix = "evd-"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is not in use

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok deleting it, I saw him not in use also before I touched cli.
deleted, done

predicate: components.NewStringFlag(predicate, "Path to the predicate, arbitrary JSON.", func(f *components.StringFlag) { f.Mandatory = true }),
predicateType: components.NewStringFlag(predicateType, "Type of the predicate.", func(f *components.StringFlag) { f.Mandatory = true }),
subjectRepoPath: components.NewStringFlag(subjectRepoPath, "Full path to some subject' location.", func(f *components.StringFlag) { f.Mandatory = false }),
subjectSha256: components.NewStringFlag(subjectSha256, "subject checksum sha256.", func(f *components.StringFlag) { f.Mandatory = false }),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here start with Upper case 'Subject'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the exec function here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use it for testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants