Skip to content

Adjusted the configuration of Maven.#532

Merged
artursouza merged 6 commits into
dapr:masterfrom
LLLLimbo:issue_531
Apr 27, 2021
Merged

Adjusted the configuration of Maven.#532
artursouza merged 6 commits into
dapr:masterfrom
LLLLimbo:issue_531

Conversation

@LLLLimbo

Copy link
Copy Markdown
Contributor

Description

Adjusted the configuration of Maven.

Issue reference

#531

@ghost

ghost commented Mar 31, 2021

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@artursouza artursouza left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please, help me understand the motivation for this PR.

Comment thread pom.xml
<version>${junit-jupiter-engine.version}</version>
</dependency>
<dependency>
<groupId>com.github.stefanbirkner</groupId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are you moving these dependencies to the parent pom? Many of these dependencies are supposed to be only in the examples pom. Does it mean all the projects will share the same dependencies?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, this does not mean that all projects will share dependencies. The dependency will only really be introduced when some project references it in dependencies.
I moved the dependencies to the dependencyManagement of the parent pom, so that the project does not have to specify a version when referring to them, and it is easy to manage.

Comment thread pom.xml Outdated
</plugins>
<extensions>
<extension>
<groupId>kr.motd.maven</groupId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why we need this plugin?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is used to detect the information about the current operating system and normalize it into more portable one.
It was an oversight on my part to add it in.

Comment thread pom.xml Outdated
</plugin>

<plugin>
<groupId>org.springframework.boot</groupId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Springboot cannot be a plugin required in the root project. Springboot is only used in the springboot project.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It seems an oversight too.

Comment thread sdk-springboot/pom.xml
<springboot.version>2.3.5.RELEASE</springboot.version>
</properties>

<dependencyManagement>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you bring this back?

Comment thread sdk-springboot/pom.xml
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.springframework</groupId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same.

Comment thread sdk-autogen/pom.xml
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.grpc</groupId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't need this anymore?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We don't need this anymore?

Its duplicated to

<dependency>
      <groupId>io.grpc</groupId>
      <artifactId>grpc-stub</artifactId>
    </dependency>

@LLLLimbo

LLLLimbo commented Apr 6, 2021

Copy link
Copy Markdown
Contributor Author

I will wait for you to confirm the following three changes before next commit.

1.Add <version>${springboot.version}</version> of spring-boot-maven-plugin to the example,otherwise maven would not find the right version.
2. Bring spring-boot-dependencies dependency management back in sdk-springboot.
3.Add maven-compiler-plugin to the parent pom's pluginManagementdapr-sdk-example need it.

And I still have a question for you, why the following exists in the dependencies of the example's pom, shouldn't it be a plugin?

<dependency>
      <groupId>com.github.os72</groupId>
      <artifactId>protoc-jar-maven-plugin</artifactId>
      <version>3.10.1</version>
</dependency>

@artursouza

Copy link
Copy Markdown
Contributor

I will wait for you to confirm the following three changes before next commit.

1.Add <version>${springboot.version}</version> of spring-boot-maven-plugin to the example,otherwise maven would not find the right version.
2. Bring spring-boot-dependencies dependency management back in sdk-springboot.
3.Add maven-compiler-plugin to the parent pom's pluginManagementdapr-sdk-example need it.

Yes to all 3 items.

And I still have a question for you, why the following exists in the dependencies of the example's pom, shouldn't it be a plugin?

<dependency>
      <groupId>com.github.os72</groupId>
      <artifactId>protoc-jar-maven-plugin</artifactId>
      <version>3.10.1</version>
</dependency>

That dependency might be wrong. Please, remove it.

LLLLimbo and others added 3 commits April 8, 2021 07:08
Add spring-boot-dependencies dependency management in sdk-springboot.
Add maven-compiler-plugin to the parent pom's pluginManagement
Remove protoc-jar-maven-plugin in  example's dependencies.
@codecov

codecov Bot commented Apr 8, 2021

Copy link
Copy Markdown

Codecov Report

Merging #532 (b012f6b) into master (a654861) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #532   +/-   ##
=========================================
  Coverage     80.65%   80.65%           
  Complexity      982      982           
=========================================
  Files            88       88           
  Lines          3024     3024           
  Branches        331      331           
=========================================
  Hits           2439     2439           
  Misses          420      420           
  Partials        165      165           

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 a654861...b012f6b. Read the comment docs.

Comment thread sdk-autogen/pom.xml

<artifactId>dapr-sdk-autogen</artifactId>
<packaging>jar</packaging>
<version>1.1.0-SNAPSHOT</version>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What happens when you run this script? https://github.com/dapr/java-sdk/blob/master/scripts/update_sdk_version.sh

We use that for our release process to automatically update the version.

@LLLLimbo LLLLimbo Apr 9, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What happens when you run this script? https://github.com/dapr/java-sdk/blob/master/scripts/update_sdk_version.sh

We use that for our release process to automatically update the version.

It says update_sdk_version.sh: line 5: $1: unbound variable.

The variable is missing here DAPR_JAVA_SDK_VERSION=$1.
What should I do to solve this problem ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pass a value, like "1.2.0"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Pass a value, like "1.2.0"

Limbo@DESKTOP-GLSB8CF MINGW64 /d/gitProject/java-sdk/scripts (master)
$ ./update_sdk_version.sh
+ DAPR_JAVA_SDK_VERSION=1.2.0
+ mvn versions:set -DnewVersion=1.2.0
[INFO] Scanning for projects...
[INFO]
[INFO] ------------------< org.apache.maven:standalone-pom >-------------------
[INFO] Building Maven Stub Project (No POM) 1
[INFO] --------------------------------[ pom ]---------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  0.291 s
[INFO] Finished at: 2021-04-20T07:18:30+08:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.codehaus.mojo:versions-maven-plugin:2.8.1:set (default-cli): Goal requires a project to execute but there is no POM in this directory (D:\gitProject\java-sdk\scripts). Please verify you invoked Maven from the correct directory. -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MissingProjectException

Works after I add cd .. before mvn versions

@artursouza artursouza merged commit 5cc7981 into dapr:master Apr 27, 2021
artursouza added a commit that referenced this pull request Apr 30, 2021
fackee pushed a commit to fackee/java-sdk that referenced this pull request Mar 28, 2022
* Adjusted the configuration of Maven.

* Update pom.xml

* Remove os-maven-plugin and spring-boot-maven-plugin in parent pom

* Add version of spring-boot-maven-plugin in the example pom.
Add spring-boot-dependencies dependency management in sdk-springboot.
Add maven-compiler-plugin to the parent pom's pluginManagement
Remove protoc-jar-maven-plugin in  example's dependencies.

* Update pom.xml

Co-authored-by: Artur Souza <artursouza.ms@outlook.com>
Co-authored-by: Mukundan Sundararajan <musundar@microsoft.com>
Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
fackee pushed a commit to fackee/java-sdk that referenced this pull request Mar 28, 2022
This reverts commit 5cc7981.

Signed-off-by: zjx244729 <zjx244729@alibaba-inc.com>
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.

2 participants