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

fix version field #166

Merged
merged 3 commits into from
May 17, 2021
Merged

fix version field #166

merged 3 commits into from
May 17, 2021

Conversation

lbh2001
Copy link
Contributor

@lbh2001 lbh2001 commented May 12, 2021

What this PR does:
Fixed version field.

Which issue(s) this PR fixes:

Fixes #138

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@@ -40,7 +40,7 @@ import (
)

// Version pixiu version
var Version = "0.1.0"
var Version = "0.2.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

0.3.0?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe v0.2.1 can merge this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xiaoliu10 Sorry for my bothering, but I've sent something about ospp program to your email, and there is also an issue I've created about that. Could you please take a look? Thank you so much! 😊

@codecov-commenter
Copy link

codecov-commenter commented May 12, 2021

Codecov Report

Merging #166 (a1940bc) into develop (118d44c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #166   +/-   ##
========================================
  Coverage    40.60%   40.60%           
========================================
  Files           35       35           
  Lines         1692     1692           
========================================
  Hits           687      687           
  Misses         903      903           
  Partials       102      102           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 118d44c...a1940bc. Read the comment docs.

@kezhenxu94
Copy link
Member

I'd rather extract the variable as a build argument, otherwise, this kind of issue is highly possible to occur next time

@lbh2001
Copy link
Contributor Author

lbh2001 commented May 12, 2021

I'd rather extract the variable as a build argument, otherwise, this kind of issue is highly possible to occur next time

So we should put variables like this into config file in the future, right? Should I fix it?

@kezhenxu94
Copy link
Member

I'd rather extract the variable as a build argument, otherwise, this kind of issue is highly possible to occur next time

So we should put variables like this into config file in the future, right? Should I fix it?

What I mean is a build argument, not configuration file, take the below as an example

https://github.com/apache/skywalking-cli/blob/d06d1ebf565756885650061ddd2e1fe527417052/cmd/swctl/main.go#L43

https://github.com/apache/skywalking-cli/blob/d06d1ebf565756885650061ddd2e1fe527417052/Makefile#L18

https://github.com/apache/skywalking-cli/blob/d06d1ebf565756885650061ddd2e1fe527417052/Makefile#L40

https://github.com/apache/skywalking-cli/blob/d06d1ebf565756885650061ddd2e1fe527417052/Makefile#L71

@kezhenxu94
Copy link
Member

A release manager should take care of the build argument when drafting a new release, and that doesn't require code changes

@lbh2001
Copy link
Contributor Author

lbh2001 commented May 12, 2021

A release manager should take care of the build argument when drafting a new release, and that doesn't require code changes

I've understood what you mean, and I will try my best to implement it after learning about Makefile. But before that, I wonder why one of the checks didn't pass and how to fix it. (Maybe this question is ridiculous 😢)

@kezhenxu94 kezhenxu94 added the chore Project chores such as CI settings, typos, etc. label May 17, 2021
@kezhenxu94 kezhenxu94 merged commit c5f9fc4 into apache:develop May 17, 2021
@kezhenxu94
Copy link
Member

Maybe this question is ridiculous

It's totally ok, there is no such question as ridiculous in community. Don't be panic

@lbh2001
Copy link
Contributor Author

lbh2001 commented May 17, 2021

Maybe this question is ridiculous

It's totally ok, there is no such question as ridiculous in community. Don't be panic

Thanks.

@xiaoliu10 xiaoliu10 added this to the v0.2.1 milestone May 29, 2021
mark4z pushed a commit that referenced this pull request Nov 7, 2021
tydhot pushed a commit to tydhot/dubbo-go-pixiu that referenced this pull request Nov 10, 2021
Former-commit-id: a367c6d [formerly c5f9fc4]
Former-commit-id: bbb6421
bobtthp pushed a commit to bobtthp/dubbo-go-pixiu that referenced this pull request Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Project chores such as CI settings, typos, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect version field
5 participants