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

Add version as input to Bitrise CLI header #827

Merged
merged 2 commits into from
Nov 17, 2022
Merged

Conversation

zsolt-vicze
Copy link
Collaborator

Use the version from the models.WorkflowRunPlan when printing the Bitrise CLI header.

@zsolt-vicze zsolt-vicze requested a review from godrei November 14, 2022 10:03
Copy link
Contributor

@tothszabi tothszabi left a comment

Choose a reason for hiding this comment

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

Nice catch. I only left a minor comment.

log/log.go Outdated
Comment on lines 148 to 150
if v == "" {
v = version.VERSION
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can drop this fallback from here because it is not the logger's responsibility to make sure the version has a non-empty value. It should just log based on the input it has received.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In that case do we need strings.TrimSpace from line 147?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you are right. That can be removed as well.

@tothszabi tothszabi merged commit 982943c into master Nov 17, 2022
@tothszabi tothszabi deleted the add-version-input branch November 17, 2022 09:45
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