Skip to content

Conversation

@suratdas
Copy link
Collaborator

This will enable SDK and clients not to send branch information. That way, it will use the default branch if not provided.

@suratdas suratdas requested a review from pashidlos December 22, 2021 21:37
const build = await this.buildsService.findOrCreate({
projectId: project.id,
branchName: createBuildDto.branchName,
branchName: (createBuildDto.branchName && createBuildDto.branchName.trim().length > 0) ? createBuildDto.branchName : project.mainBranchName,
Copy link
Member

Choose a reason for hiding this comment

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

empty branch name should be validated in another place if needed

Suggested change
branchName: (createBuildDto.branchName && createBuildDto.branchName.trim().length > 0) ? createBuildDto.branchName : project.mainBranchName,
branchName: createBuildDto.branchName ?? project.mainBranchName,

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

@suratdas suratdas Dec 24, 2021

Choose a reason for hiding this comment

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

Thanks. Added. I also made branchName optional in the DTO just like ciBuildId.

suratdas and others added 2 commits December 24, 2021 12:37
Co-authored-by: Pavel Strunkin <pashidlos@gmail.com>
@suratdas suratdas requested a review from pashidlos December 24, 2021 20:47
Copy link
Member

@pashidlos pashidlos left a comment

Choose a reason for hiding this comment

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

LGTM!

@pashidlos
Copy link
Member

@pashidlos pashidlos merged commit 4b755c3 into master Dec 26, 2021
@pashidlos pashidlos deleted the take-main-branch branch December 26, 2021 09:44
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