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

refactoring for SkyApm.Transport.Protocol #584

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

lu-xiaoshuang
Copy link
Contributor

  • Why submit this pull request?
  • [*] refactoring

@wu-sheng
Copy link
Member

Please fix the CI.

@lu-xiaoshuang
Copy link
Contributor Author

CI issue fixed

@wu-sheng wu-sheng added this to the 2.3.0 milestone Mar 27, 2024
@wu-sheng
Copy link
Member

Mark this on 2.3.0, as @liuhaoyang is planning to run 2.2.0 release, we should stop adding new things.

@lu-xiaoshuang
Copy link
Contributor Author

Hi @liuhaoyang , would you please review this pull request?

@wu-sheng
Copy link
Member

Why do we need this? Please explain your proposal.

@wu-sheng wu-sheng requested a review from liuhaoyang June 24, 2024 01:49
@lu-xiaoshuang
Copy link
Contributor Author

Why do we need this? Please explain your proposal.

reporter interface layer needs an independent dotnet project. Both gRPC reporter and Kafka reporter are concrete implementations which live in their own project directories.

@wu-sheng
Copy link
Member

We don't have Kafka reporter AFAIK. Nother to be shared.

@lu-xiaoshuang
Copy link
Contributor Author

We don't have Kafka reporter AFAIK. Nother to be shared.

#559

@wu-sheng
Copy link
Member

That thing had no progress for over half of the year. So, I closed that.

I don't know what happened.

@lu-xiaoshuang
Copy link
Contributor Author

That thing had no progress for over half of the year. So, I closed that.

I don't know what happened.

What about this plan for kafka reporter?

  1. merge refactoring for SkyApm.Transport.Protocol #584 @wu-sheng
  2. reopen Add kafka reporter #559 @wu-sheng
  3. update Add kafka reporter #559 @lu-xiaoshuang
  4. merge Add kafka reporter #559 @liuhaoyang

@wu-sheng
Copy link
Member

All others are closed, how close are they ready to be accepted? @liuhaoyang

@liuhaoyang liuhaoyang merged commit ed1d377 into SkyAPM:main Jun 25, 2024
2 checks passed
@wu-sheng
Copy link
Member

@lu-xiaoshuang We have merged this, please update reporter part.

@lu-xiaoshuang
Copy link
Contributor Author

@lu-xiaoshuang We have merged this, please update reporter part.

Copy. Please reopen #559 ~

@wu-sheng
Copy link
Member

You could open a new one with a clear context and including this change.

@lu-xiaoshuang
Copy link
Contributor Author

You could open a new one with a clear context and including this change.

okay.

@lu-xiaoshuang lu-xiaoshuang deleted the refactoring branch June 25, 2024 10:31
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