-
Notifications
You must be signed in to change notification settings - Fork 638
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
[ISSUE #4521] A poor naming. #4523
Conversation
@pandaapo Please review my PR and suggest changes if there exists any. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to the Apache EventMesh community!!
This is your first PR in our project. We're very excited to have you onboard contributing. Your contributions are greatly appreciated!
Please make sure that the changes are covered by tests.
We will be here shortly.
Let us know if you need any help!
Want to get closer to the community?
WeChat Assistant | WeChat Public Account | Slack |
---|---|---|
Join Slack Chat |
Mailing Lists:
Name | Description | Subscribe | Unsubscribe | Archive |
---|---|---|---|---|
Users | User support and questions mailing list | Subscribe | Unsubscribe | Mail Archives |
Development | Development related discussions | Subscribe | Unsubscribe | Mail Archives |
Commits | All commits to repositories | Subscribe | Unsubscribe | Mail Archives |
Issues | Issues or PRs comments and reviews | Subscribe | Unsubscribe | Mail Archives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your modifications were a bit bad. And you haven't checked any other references to this class.
Am i allowed to do it @pandaapo ? |
You can rollback or change to another appropriate name. In addition, you need to check other references to this class. |
The usage of this exception enumeration class has already undergone some changes in my development branch. Renaming it based on the existing master branch would make it difficult to merge my branch. If you want this issue to be fixed as soon as possible, I can submit a portion of the code first. If you're not in a hurry, you can see these changes in the PR of #4501. @pandaapo 这个异常枚举类的用法在我的开发分支中已经发生了一些改变,根据现有的master分支进行重命名,会导致我的分支合入困难。如果您希望这个问题被尽快修复,我可以先提交一部分代码。如果您不着急的话,您会在 #4501 的PR中见到这些更改。@pandaapo |
@pandaapo I have done all the changes. Please review my PR |
@Asymtode712 Hi, welcome. I'm sorry, but the changes you're making conflict significantly with my development work. You might want to consider working on a different good first issue. Additionally, editing code in this way using GitHub's online editor is not recommended. It's better to use an IDE that supports the Java language for code refactoring. |
@Pil0tXia I have discovered all the occurences using VS Code only. I have just replaced them using Github's editor. |
@Asymtode712 I'm sorry that this PR doesn't correspond with EventMeshAdmin's development plan and it conflicts a lot. I'm the author of |
The issue is not urgent, but I hope you can submit a portion of the code about it first. As Asymtode712 has already submitted PR, I will review this PR when I have time. If there is no better PR and there are no problems with this PR, there is no reason not to approve it. Please understand. Thank you. 该issue不是很急,但是我希望你能先提交关于该issue的代码。由于Asymtode712 已经提交,有空的时候我将审核该PR。如果没有更好的PR,同时该PR没有任何问题,我就没有理由不通过该PR。请理解,谢谢。 |
I will take some time to review the relevant PRs after work today. |
Codecov Report
@@ Coverage Diff @@
## master #4523 +/- ##
=========================================
Coverage 15.91% 15.91%
Complexity 1545 1545
=========================================
Files 730 730
Lines 28896 28896
Branches 2745 2745
=========================================
Hits 4600 4600
Misses 23816 23816
Partials 480 480 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
PR #4524 is better, so I will approve it. Thank you all the same for your work and sorry about to close this. You can try other 'good first issues' that have not been claimed or processed by anyone yet. |
Fixes #4521
Motivation
Explain the content here.
Explain why you want to make the changes and what problem you're trying to solve.
Modifications
Describe the modifications you've done.
Documentation