-
Notifications
You must be signed in to change notification settings - Fork 26.4k
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
feat:remove jprotoc #13145
feat:remove jprotoc #13145
Conversation
Codecov Report
@@ Coverage Diff @@
## 3.3 #13145 +/- ##
============================================
- Coverage 68.41% 68.03% -0.38%
Complexity 8 8
============================================
Files 1718 1726 +8
Lines 70833 71239 +406
Branches 10238 10287 +49
============================================
+ Hits 48460 48470 +10
- Misses 17683 18078 +395
- Partials 4690 4691 +1 see 39 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
LGTM
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.apache.dubbo.maven.plugin.protoc.plugin; |
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.
Remove a package path, and org.apache.dubbo.maven.plugin.protoc
is better.
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.
Already refactored
@sadfera This change should also be accompanied by a change in the documentation used. |
Will be updated later on dubbo-website |
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.
Update on 3.3 branch would be better
|
dubbo-compiler/README.md
Outdated
<plugin> | ||
<groupId>org.apache.dubbo</groupId> | ||
<artifactId>dubbo-maven-plugin</artifactId> | ||
<version>3.2.7-SNAPSHOT</version> |
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.
<version>3.2.7-SNAPSHOT</version> | |
<version>3.3.0</version> |
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to You under the Apache License, Version 2.0 | ||
* (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ |
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.
Remove this header and add this file into exclustion in license checker
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.
ok
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
public class DubboProtocPluginWrapperFactory { |
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.
No place to use this class?
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.
org.apache.dubbo.maven.plugin.protoc.DubboProtocCompilerMojo use it , and It will help me find the pluginWrapper class based on the operating system
NOTICE
Outdated
Maven Protocol Buffers Plugin includes a binary distribution of WinRun4J, | ||
which is used to execute protoc plugins written in Java | ||
under 32-bit and 64-bit Windows operating systems. | ||
|
||
WinRun4J is licensed under the Common Public License (CPL). | ||
http://www.eclipse.org/legal/cpl-v10.html | ||
|
||
Additional information about WinRun4J is available on | ||
the project's home page: | ||
http://winrun4j.sourceforge.net/ | ||
|
||
For information about the bundled versions of the binaries | ||
please read src/main/resources/winrun4j/README.txt | ||
inside this git repository. |
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.
These lines can be removed
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.
ok,fixed
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.
LGTM
Kudos, SonarCloud Quality Gate passed! |
What is the purpose of the change
dubbo-compiler remove jprotoc dependecy
Brief changelog
1.Remove jprotc dependency and add mustache dependency for template generation code
2.Retain the ProtoTypeMap provided by the original jprotc for storing message types parsed from proto
3.Remove ProtocPlugin and use Dubbo's own DubboProtocPlugin as the main entry point for protoc plugin calls
4.DubboProtocPlugin no longer provides debug methods and only supports FEATURE_ PROTO3_ OPTIONAL
5.dubbo-maven-plugin supported dubbo-protoc-compiler mojo
Verifying this change
Checklist