Skip to content

initial commit #3022

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

initial commit #3022

wants to merge 1 commit into from

Conversation

fluxxBot
Copy link
Collaborator

@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
Collaborator

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
Collaborator

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
Collaborator

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
Collaborator

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
Collaborator

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
Collaborator

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
Collaborator

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)
}
}

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