Skip to content

Conversation

@fluxxBot
Copy link
Contributor

@fluxxBot fluxxBot commented Jul 3, 2025

  • All tests have passed. If this feature is not already covered by the tests, new tests have been added.
  • The pull request is targeting the dev branch.
  • The code has been validated to compile successfully by running go vet ./....
  • The code has been formatted properly using go fmt ./....

Comment on lines +125 to +128
vConfig, err := project.ReadConfigFile(configFilePath, project.YAML)
if err != nil {
return nil, err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can validate config file contents before proceeding.

if err != nil {
return err
}
err = build.AddArtifacts(moduleName, "generic", artifacts...)
Copy link
Contributor

Choose a reason for hiding this comment

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

Some Improvements:

  • We can add error context if adding artifacts fails.
  • We can add logging for successful artifact addition.

}

func getBuildPropsForArtifact(buildName, buildNumber, project string) (string, error) {
err := buildUtils.SaveBuildGeneralDetails(buildName, buildNumber, project)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider logging if saving general details fails.

}
searchReader, err = servicesManager.SearchFiles(searchParams)
if err != nil {
log.Error("Failed to get uploaded npm package: ", err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

Error message is specific to npm, but the function is generic.
We can update it to generic message: "Failed to get uploaded package: ..."

if err != nil {
log.Warn("Unable to set build properties: ", err, "\nThis may cause build to not properly link with artifact, please add build name and build number properties on the tarball artifact manually")
}
buildInfoArtifacts, err := utils.ConvertArtifactsSearchDetailsToBuildInfoArtifacts(searchReader)
Copy link
Contributor

Choose a reason for hiding this comment

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

if needed we can check for errors after converting artifacts before proceeding

Comment on lines +31 to +42
func GetBuildInfoForUploadedArtifacts(uploadedFile string, buildConfiguration *buildUtils.BuildConfiguration) error {
repoConfig, err := extractRepositoryConfig()
if err != nil {
return err
}
serverDetails, err := repoConfig.ServerDetails()
if err != nil {
return err
}
err = saveBuildInfo(serverDetails, repoConfig.TargetRepo(), uploadedFile, buildConfiguration)
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logs and the error messages can be updated,

func GetBuildInfoForUploadedArtifacts(uploadedFile string, buildConfiguration *buildUtils.BuildConfiguration) error {
	log.Info("Extracting repository configuration for build info...")
	repoConfig, err := extractRepositoryConfig()
	if err != nil {
		log.Error("Failed to extract repository configuration: ", err)
		return fmt.Errorf("extractRepositoryConfig failed: %w", err)
	}

	log.Info("Retrieving server details...")
	serverDetails, err := repoConfig.ServerDetails()
	if err != nil {
		log.Error("Failed to retrieve server details: ", err)
		return fmt.Errorf("ServerDetails extraction failed: %w", err)
	}

	log.Infof("Saving build info for uploaded file: %s", uploadedFile)
	err = saveBuildInfo(serverDetails, repoConfig.TargetRepo(), uploadedFile, buildConfiguration)
	if err != nil {
		log.Error("Failed to save build info: ", err)
		return fmt.Errorf("saveBuildInfo failed: %w", err)
	}

	log.Info("Successfully saved build info for uploaded artifact.")
	return nil
}

Comment on lines +176 to +179
err = cliutils.CreateConfigCmd(ctx, project.FromString(confType))
if err != nil {
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
err = cliutils.CreateConfigCmd(ctx, project.FromString(confType))
if err != nil {
return err
}
if !configExists(confType) {
err = cliutils.CreateConfigCmd(ctx, project.FromString(confType))
if err != nil {
return fmt.Errorf("failed to create config: %w", err)
}
}

@RemiBou RemiBou added the jfrog-internal Items created by jfrog employees label Oct 17, 2025
@RemiBou
Copy link
Contributor

RemiBou commented Oct 20, 2025

CLosing as not needed anymore

@RemiBou RemiBou closed this Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jfrog-internal Items created by jfrog employees

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants