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

Add Code Formatter #85

Closed
wants to merge 2 commits into from
Closed

Add Code Formatter #85

wants to merge 2 commits into from

Conversation

maarzt
Copy link
Contributor

@maarzt maarzt commented Mar 7, 2019

This PR configures a maven code format plugin in pom-scijava. A maven project that depends on pom-scijava can be formatted according to ImageJ Coding Style by simply calling:

$ mvn formatter:format

This will use the Eclipse coding style configuration files in this (not yet released) maven artifact:
https://github.com/scijava/scijava-coding-style

It can also be set up to use ImgLib2 or SCIFIO coding style by stetting by setting the maven property to "imglib2" or "scifio" ("imagej" is the default value):

<properties>
    <coding-style>imglib2</coding-style>
</properties>

Sorting of java imports is supported as well:

$ mvn impsort:sort

WIP: This depends on an other maven artifact scijava-coding-style
which is not released yet.
@maarzt
Copy link
Contributor Author

maarzt commented Mar 7, 2019

Before merge: Please move this dependency https://github.com/maarzt/scijava-coding-style to the SciJava organization, "Travisify" and release. And then change the WIP commit to point to the correct version.

@imagejan
Copy link
Member

@maarzt wrote:

Please move this dependency maarzt/scijava-coding-style to the SciJava organization

Since you're a member of the scijava organization, you should be able to move the repo from maarzt to scijava.
We can then adapt the pom.xml accordingly, to have everything ready until @ctrueden finds time to review it 🙂 .

@maarzt
Copy link
Contributor Author

maarzt commented Mar 25, 2019

@imagejan wrote

Since you're a member of the scijava organization, you should be able to move the repo from maarzt to scijava.

I tried, but github tells me, that I don't have permission to create a repository in the scijava organization :-/

@imagejan
Copy link
Member

Alright, sorry, my guess was wrong. For the SciJava org, repository creation is disabled for Members. I see two ways out of this:

  • Allow creation of (public and private) repositories for all members of the scijava org.
  • Make @maarzt an Owner of the scijava org.

While I technically can put either of these in action, I'd like to ask @ctrueden for his opinion, because I don't want to mess with the permissions of such an important organization 🙂

Alternatively, @maarzt, you can make me a co-admin of maarzt/scijava-coding-style and I can try to move it for you.

@ctrueden
Copy link
Member

It's fine to allow creation of new repositories in the scijava org. As long as everyone does it sparingly!

@imagejan
Copy link
Member

Alright, I changed the scijava settings so that members can now create repositories.
@maarzt can you try again moving the repository?

@maarzt
Copy link
Contributor Author

maarzt commented Mar 26, 2019

@imagejan I moved the repository to the scijava organization. We can now work on the pom.
What do you think needs to change?

@imagejan
Copy link
Member

imagejan commented Mar 26, 2019

@maarzt most of the changes I had in mind you covered in scijava/scijava-coding-style@c9dd725 already. The remaining things are:

  • Use travis-ci.com instead of the legacy .org
  • Remove now-redundant group ID org.scijava

I did this in scijava/scijava-coding-style#1, feel free to merge or add changes.

Next steps:

  • Activate the repository on travis-ci.com travis-ci.org
  • Run travisify.sh (while logged in to travis-ci.com with the CLI client)
  • Run release-version.sh

@imagejan
Copy link
Member

For some reason I cannot activate scijava/scijava-coding-style on Travis CI (neither on .com nor on .org). @ctrueden could you have a look if that works for you?

@ctrueden
Copy link
Member

Nope.

travisify.sh -f
$ travisify.sh -f
- Repository = scijava/scijava-coding-style
- Detected domain from pom.xml: travis-ci.com
- Creating .travis.yml
[master 83dbdb9] Travis: update .travis.yml
 1 file changed, 11 insertions(+)
 create mode 100644 .travis.yml
- Creating .travis/build.sh
[master 0cde327] Travis: update .travis/build.sh
 1 file changed, 3 insertions(+)
 create mode 100755 .travis/build.sh
- Version of pom-scijava (25.0.0) is OK
- Adding <releaseProfiles> property
- Updating pom.xml
[master 14437f5] POM: deploy releases to the ImageJ repository
 1 file changed, 3 insertions(+)
- Adding Travis badge to README.md
- Updating README.md
[master 7f0335b] Travis: add badge to README.md
 1 file changed, 2 insertions(+)
- Encrypting GPG_KEY_NAME
repository not known to https://api.travis-ci.com/: scijava/scijava-coding-style
[ERROR] Failed to encrypt variable 'GPG_KEY_NAME=...'

And https://travis-ci.com/scijava/scijava-coding-style says "We couldn't find the repository
scijava/scijava-coding-style" currently.

Let's try again tomorrow. If it still doesn't work, contact Travis support? Or we can just punt and activate it on travis-ci.org instead. I've had to do that for a couple of other repositories in past months.

@imagejan
Copy link
Member

I tried again and still wasn't able to activate on travis-ci.com, so now activated it on travis-ci.org instead.

The build reports to have failed (there seem to have been issues with the gpg encryption?!), but actually was built and deployed:
http://maven.imagej.net/#nexus-search;quick~scijava-coding-style

@ctrueden
Copy link
Member

The issue with scijava-coding-style was with decrypting the GPG key—which is not used anyway when deploying to the SciJava Maven repository.

I have re-Travisified the repository, which has fixed the issue, in case we ever switch to OSS Sonatype in the future (since OSS Sonatype requires GPG signing).

The build failed again due to the switch to maven.scijava.org, but I will have that tidied up shortly.

@ctrueden
Copy link
Member

ctrueden commented Apr 30, 2019

Regarding the scijava-coding-style repository, all is well now. I have cut the 1.0.0 release, which if all goes well will build and deploy momentarily. (Edit: It did not deploy successfully, due to some misconfiguration of maven.scijava.org. I'm working on it. Will rebuild the tag once fixed.)

This PR still needs a little TLC, however:

  1. lt has a WIP commit.
  2. I want to use scijava.coding-style instead of coding-style for the property name.
  3. We cannot depend on a SNAPSHOT of scijava-coding-style—it needs to be 1.0.0.
  4. Artifact versions, including scijava-coding-style and impsort-maven-plugin, should be declared and referenced as properties, to make it easier to override them for testing etc.
  5. We should strongly consider pushing this feature into pom-scijava-base, rather than pom-scijava, since it is a build system feature rather than a bill-of-materials-related feature.

@ctrueden
Copy link
Member

Finally got scijava-coding-style 1.0.0 released.

@maarzt I decided to redo this patch in pom-scijava-base myself. Merged to master now. Thanks so much for working on it! 👍

I will test it along with pom-scijava melting-pot work later this week, before releasing pom-scijava-base 7.0.0, and correct any issues with it. Though if anyone else wants to test it as well, please do.

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.

None yet

3 participants