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

Rename crate name skywalking_rust to skywalking? #23

Merged
merged 2 commits into from
Jul 3, 2022

Conversation

jmjoy
Copy link
Member

@jmjoy jmjoy commented Jul 3, 2022

The suffix _rust is not necessary, and using the crate name skywalking can prevent fraudulent use by third parties.

@codecov-commenter
Copy link

codecov-commenter commented Jul 3, 2022

Codecov Report

Merging #23 (b74f993) into master (25e4f28) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #23   +/-   ##
=======================================
  Coverage   86.91%   86.91%           
=======================================
  Files           9        9           
  Lines         298      298           
=======================================
  Hits          259      259           
  Misses         39       39           
Impacted Files Coverage Δ
src/context/trace_context.rs 74.78% <ø> (ø)
src/reporter/grpc.rs 0.00% <ø> (ø)
tests/propagation.rs 96.77% <ø> (ø)
tests/trace_context.rs 100.00% <ø> (ø)

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 25e4f28...b74f993. Read the comment docs.

@wu-sheng
Copy link
Member

wu-sheng commented Jul 3, 2022

It works for me, but we have to release 0.2.0 to make this works.

@wu-sheng wu-sheng added this to the 0.2.0 milestone Jul 3, 2022
@wu-sheng wu-sheng added the chore Housekeeping things like CI, license things label Jul 3, 2022
@wu-sheng
Copy link
Member

wu-sheng commented Jul 3, 2022

We should keep this hold when we are going to release 0.2.0 right away. Otherwise, the readme doc would look like wrong for the end-users. Right?

Comment on lines +36 to +37
use skywalking::context::trace_context::TracingContext;
use skywalking::reporter::grpc::Reporter;
Copy link
Member

Choose a reason for hiding this comment

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

The compiling doc in the readme is still skywalking-rust, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Compiling doc is means the doc generated by cargo doc? With 0.2.0 released, the skywalking will be a new crate and the name is skywalking in docs.rs.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I didn't mean this is wrong. I mean https://github.com/apache/skywalking-rust#how-to-compile doesn't match here.

@jmjoy
Copy link
Member Author

jmjoy commented Jul 3, 2022

OK, but I think the crate name skywalking should be preempted first to prevent it from being occupied by others.

@wu-sheng
Copy link
Member

wu-sheng commented Jul 3, 2022

In Apache, we have to release it officially(a new tag approved by the PMC), then I could publish it to the cargo.

Do you think it is a good time to release 0.2.0? And we move capture/continuous to 0.3.0?

@jmjoy
Copy link
Member Author

jmjoy commented Jul 3, 2022

In Apache, we have to release it officially(a new tag approved by the PMC), then I could publish it to the cargo.

Do you think it is a good time to release 0.2.0? And we move capture/continuous to 0.3.0?

I think it is OK, or use alpha version like 0.2.0-alpha.1 ?

@wu-sheng
Copy link
Member

wu-sheng commented Jul 3, 2022

In Apache, we have to release it officially(a new tag approved by the PMC), then I could publish it to the cargo.
Do you think it is a good time to release 0.2.0? And we move capture/continuous to 0.3.0?

I think it is OK, or use alpha version like 0.2.0-alpha.1 ?

I think it is fine if 0.2.0 APIs would be changed in 0.3.0. We usually just make sure 1.0 as a stable version.

@jmjoy
Copy link
Member Author

jmjoy commented Jul 3, 2022

In Apache, we have to release it officially(a new tag approved by the PMC), then I could publish it to the cargo.
Do you think it is a good time to release 0.2.0? And we move capture/continuous to 0.3.0?

I think it is OK, or use alpha version like 0.2.0-alpha.1 ?

I think it is fine if 0.2.0 APIs would be changed in 0.3.0. We usually just make sure 1.0 as a stable version.

Yes, so we can use 0.2.0.

@wu-sheng
Copy link
Member

wu-sheng commented Jul 3, 2022

@jmjoy Are you in the slack or QQ group? Or could you add me wechat through @heyanlong? (I think you two know each other?)

I may need your help to release 0.2.0, as you are not a committer yet. This library was released by @Shikugawa , who is busy in his daily work.

@jmjoy
Copy link
Member Author

jmjoy commented Jul 3, 2022

@jmjoy Are you in the slack or QQ group? Or could you add me wechat through @heyanlong? (I think you two know each other?)

I may need your help to release 0.2.0, as you are not a committer yet. This library was released by @Shikugawa , who is busy in his daily work.

I'm not in QQ group, I just have the QQ of heyanlong, maybe you can give me the QQ gorup number?

@wu-sheng
Copy link
Member

wu-sheng commented Jul 3, 2022

I just added you through QQ on your profile page.

@wu-sheng wu-sheng merged commit e1e4cb5 into apache:master Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Housekeeping things like CI, license things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants