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

Update dependencies, add instructions to build and a GitHub Action #37

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Aug 8, 2021

To allow developers without gpg test their changes I moved signing under a flag. Now if you need to sign artefacts you should pass -DperformRelease=true flag to the maven command:

mvn -DperformRelease=true deploy

@Mingun Mingun changed the title Update dependencies, add instructions to build and GitHub Action Update dependencies, add instructions to build and a GitHub Action Aug 8, 2021
Copy link
Member

@generalmimon generalmimon left a comment

Choose a reason for hiding this comment

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

To allow developers without gpg test their changes I moved signing under a flag. Now if you need to sign artefacts you should pass -DperformRelease=true flag to the maven command

Thanks for that, I also needed to comment the maven-gpg-plugin in pom.xml locally to be able to build the Maven package:

-      <plugin>
+      <!-- <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-gpg-plugin</artifactId>
         <version>1.5</version>
         <executions>
           <execution>
             <id>sign-artifacts</id>
             <phase>verify</phase>
             <goals>
               <goal>sign</goal>
             </goals>
           </execution>
         </executions>

-      </plugin>
+      </plugin> -->

So it'll be a nice improvement that I won't have to do that anymore.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
pom.xml Outdated
Comment on lines 50 to 96
<version>2.2.1</version>
<version>3.2.1</version>
Copy link
Member

Choose a reason for hiding this comment

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

Are there any breaking changes, migration guides etc. for these major version bumps? If these plugins follow SemVer, we have to assume that there are some incompatible API changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change log is too long to read (128 commits) but it seems it contains mostly infrastructure changes. I think the best way is just check the output -- it shouldn't be too hard. At least it generates doc and they seems correct.

The most important thing in 2.x -> 3.x migration seems to be summarized in this phrase from the plugin's page:

Starting with version 3.0.0 of the plugin all properties which could be used via command line have been named based on the following schema maven.source.*. Further details can be found in the goal documentations.

As I see we don't use CLI, so that does not influence us.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@generalmimon, could you comment on this?

README.md Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
Comment on lines 33 to 34
- name: Run the Maven verify phase
run: mvn --batch-mode --update-snapshots verify
Copy link
Member

Choose a reason for hiding this comment

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

What does this command actually do? Is it some simulation of the release process, so that we can see errors that would block the release and fix them in advance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy this from the GitHub Action docs. I think that those guys known the idiomatic way, but here explanations of the switches:

  • --batch-mode -- do not request any interactive input if it can arise
  • --update-snapshots -- needed only if we have dependencies bu we don't. Will remove
  • verify -- runs almost all phases of the project except actual installing into local repository and deploying to the remote repository. It seems ideal for tests

Comment on lines 28 to 19
- uses: actions/setup-java@v2
with:
java-version: '11'
distribution: 'adopt'
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would make sense to use strategy matrix to test on multiple Java versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if we need that for such a simple project...

@Mingun
Copy link
Contributor Author

Mingun commented Oct 26, 2021

So what the status?

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