Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add Code Formatter #85

Closed
maarzt wants to merge 2 commits intoscijava:master from
maarzt:code-formatter
Closed

Add Code Formatter #85
maarzt wants to merge 2 commits intoscijava:master from
maarzt:code-formatter

Conversation

@maarzt
Copy link
Contributor

@maarzt maarzt commented Mar 7, 2019
edited
Loading

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

imagejan reacted with heart emoji
maarzt added 2 commits March 7, 2019 16:40
WIP: This depends on an other maven artifact scijava-coding-style
which is not released yet.
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.

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

ctrueden reacted with thumbs up emoji

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 :-/

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 reacted with thumbs up emoji

Copy link
Member

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

imagejan reacted with thumbs up emoji

Copy link
Member

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

Copy link
Contributor Author

maarzt commented Mar 26, 2019
edited
Loading

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

Copy link
Member

imagejan commented Mar 26, 2019
edited
Loading

@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

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?

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.

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 reacted with thumbs up emoji

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.

imagejan reacted with thumbs up emoji

Copy link
Member

ctrueden commented Apr 30, 2019
edited
Loading

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.
imagejan reacted with heart emoji

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.

imagejan and maarzt reacted with thumbs up emoji maarzt reacted with hooray emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /