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

Create Build Evidence. #16

Merged
merged 2 commits into from
Aug 4, 2024
Merged

Create Build Evidence. #16

merged 2 commits into from
Aug 4, 2024

Conversation

osaidwtd
Copy link
Contributor

No description provided.

build string
}

func NewCreateEvidenceBuild(serverDetails *coreConfig.ServerDetails, predicateFilePath string, predicateType string, key string, keyId string,

Choose a reason for hiding this comment

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

Optional: as the types are the same, the string may be put just once:

func NewCreateEvidenceBuild(serverDetails *coreConfig.ServerDetails, 
	predicateFilePath, predicateType, key, keyId, project, build string) Command {

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 (c *createEvidenceBuild) buildBuildInfoSubjectPath(artifactoryClient artifactory.ArtifactoryServicesManager) (string, error) {
build := strings.Split(c.build, ":")

Choose a reason for hiding this comment

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

Please, raise the question about the input parameters one more time.
This split means that neither Build Name nor Build Number may contain : (this is not the case).
Hence, it means that we do not support Builds with name/number having :.
Needs to be confirmed explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I talked with Noam, there is a chance to change all other operations to be splitter to --name --version.
will take it in a separate Jira ticket.

BuildNumber: number,
ProjectKey: project,
}
res, b, err := artifactoryClient.GetBuildInfo(buildInfo)

Choose a reason for hiding this comment

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

Please, rename b to something like ok.

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

res, b, err := artifactoryClient.GetBuildInfo(buildInfo)
if !b {
errorMessage := fmt.Sprintf("failed to find build, name:%s, number:%s, project: %s", name, number, project)
if err != nil {

Choose a reason for hiding this comment

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

The order of conditions looks wrong, the err should be checked first:

if err != nil {
    return "", err
}
if !ok {
    errorMessage := fmt.Sprintf("failed to find build, name:%s, number:%s, project: %s", name, number, project)
    return "", errorutils.CheckErrorf(errorMessage)
}

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

@osaidwtd
Copy link
Contributor Author

osaidwtd commented Aug 4, 2024

Create Evidence Build-Info Using CLI

@osaidwtd osaidwtd merged commit 6349e2e into jfrog:main Aug 4, 2024
1 of 6 checks passed
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.

2 participants