feat:remove jprotoc by sadfera · Pull Request #13145 · apache/dubbo

sadfera · 2023-09-30T12:38:53Z

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

Make sure there is a GitHub_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
Add some description to dubbo-website project if you are requesting to add a feature.
GitHub Actions works fine on your own branch.
If this contribution is large, please follow the Software Donation Guide.

codecov-commenter · 2023-10-05T03:25:37Z

Codecov Report

Merging #13145 (3219783) into 3.3 (c98c487) will decrease coverage by 0.38%.
The diff coverage is n/a.

@@             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

guohao

LGTM

CrazyHZM · 2023-10-07T02:16:42Z

...c/main/java/org/apache/dubbo/maven/plugin/protoc/plugin/DubboProtocPluginWrapperFactory.java

+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.maven.plugin.protoc.plugin;


Remove a package path, and org.apache.dubbo.maven.plugin.protoc is better.

Already refactored

CrazyHZM · 2023-10-07T02:19:23Z

@sadfera This change should also be accompanied by a change in the documentation used.

sadfera · 2023-10-07T03:11:37Z

@sadfera This change should also be accompanied by a change in the documentation used.

Will be updated later on dubbo-website

AlbumenJ

Update on 3.3 branch would be better

sadfera · 2023-10-10T03:06:56Z

Update on 3.3 branch would be better
ok

AlbumenJ · 2023-10-10T08:14:36Z

dubbo-compiler/README.md

+  <plugin>
+    <groupId>org.apache.dubbo</groupId>
+    <artifactId>dubbo-maven-plugin</artifactId>
+    <version>3.2.7-SNAPSHOT</version>


Suggested change

<version>3.2.7-SNAPSHOT</version>

<version>3.3.0</version>

AlbumenJ · 2023-10-10T08:15:17Z

dubbo-compiler/src/main/java/org/apache/dubbo/gen/utils/ProtoTypeMap.java

+/*
+ * 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.
+ */


Remove this header and add this file into exclustion in license checker

CrazyHZM · 2023-10-13T04:10:14Z

...ugin/src/main/java/org/apache/dubbo/maven/plugin/protoc/DubboProtocPluginWrapperFactory.java

+import java.util.HashMap;
+import java.util.Map;
+
+public class DubboProtocPluginWrapperFactory {


No place to use this class?

org.apache.dubbo.maven.plugin.protoc.DubboProtocCompilerMojo use it , and It will help me find the pluginWrapper class based on the operating system

.licenserc.yaml

AlbumenJ · 2023-10-17T08:54:26Z

NOTICE

+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.


These lines can be removed

EarthChen

LGTM

…emove_jprotoc

sonarqubecloud · 2023-10-19T04:17:38Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
66 Code Smells

0.0% Coverage
0.0% Duplication

sadfera added 4 commits September 30, 2023 20:34

feat:remove jprotoc

ebc4bc1

fix:add dependency and license

cd80b21

fix:remove useless import

a7efb3e

fix:checkstyle bug

88a1017

guohao approved these changes Oct 7, 2023

View reviewed changes

CrazyHZM requested changes Oct 7, 2023

View reviewed changes

sadfera added 2 commits October 7, 2023 10:57

fix:SonarCloud bug

aeafe40

refactor:remove a package path

8d174d7

sadfera and others added 2 commits October 7, 2023 17:07

fix:handle InterruptedException

a0cdd11

Merge branch '3.2' into feat_remove_jprotoc

16a6122

AlbumenJ reviewed Oct 10, 2023

View reviewed changes

sadfera changed the base branch from 3.2 to 3.3 October 10, 2023 02:47

Merge branch '3.3' into feat_remove_jprotoc

4327940

Merge branch '3.3' into feat_remove_jprotoc

93b0e7c

AlbumenJ requested changes Oct 10, 2023

View reviewed changes

sadfera added 2 commits October 10, 2023 16:59

refactor:modify license and format code

21c3532

fix:checkstyle excluded

abd314c

sadfera requested a review from CrazyHZM October 10, 2023 10:37

Merge branch '3.3' into feat_remove_jprotoc

13747b7

CrazyHZM reviewed Oct 13, 2023

View reviewed changes

AlbumenJ requested changes Oct 13, 2023

View reviewed changes

.licenserc.yaml Show resolved Hide resolved

Add NOTICE

19d4b0e

AlbumenJ requested changes Oct 17, 2023

View reviewed changes

fix:remove unless notice

8cc35da

AlbumenJ approved these changes Oct 18, 2023

View reviewed changes

Merge branch '3.3' into feat_remove_jprotoc

ad1db84

EarthChen approved these changes Oct 18, 2023

View reviewed changes

pengxiaoyu and others added 4 commits October 18, 2023 20:03

refactor:code format

0c00271

Merge remote-tracking branch 'origin/feat_remove_jprotoc' into feat_r…

2587d4a

…emove_jprotoc

Merge branch '3.3' into feat_remove_jprotoc

6b04b07

Merge branch '3.3' into feat_remove_jprotoc

3219783

EarthChen merged commit d0c41f5 into apache:3.3 Oct 19, 2023

Comments

Conversation

sadfera commented Sep 30, 2023

What is the purpose of the change

Brief changelog

Verifying this change

Checklist

Uh oh!

codecov-commenter commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

guohao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CrazyHZM commented Oct 7, 2023

Uh oh!

sadfera commented Oct 7, 2023

Uh oh!

AlbumenJ left a comment

Choose a reason for hiding this comment

Uh oh!

sadfera commented Oct 10, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sadfera Oct 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EarthChen left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-commenter commented Oct 5, 2023 •

edited

Loading

sadfera Oct 13, 2023 •

edited

Loading