Uploading a bug fix to Fiji

github
contributing
Tags: #<Tag:0x00007fd69c3ec298> #<Tag:0x00007fd69c3ebe10>

#1

Hi, I got a report of a bug in the Auto Local Threshold plugin and solved it.
I have Fiji installed in ~/Fiji.app/

First I cloned the source of the plugin:

git clone https://github.com/fiji/Auto_Threshold

Fixed the bug, then I compiled it with mvn and it was successful
Finally I run as suggested in:

mvn -Dimagej.app.directory=$HOME/Fiji.app/ -Ddelete.other.versions=true

Restarted Fiji and the new plugin is there. All good.

Now how do I push the modifications to github?
I am happy to send the java file to anybody if it is too complicated. :-/


#2

The preferred way is to push to a topic branch in a forked version of the Auto_Threshold repository and then open a pull request (see also Working with Pull Requests and Contributing on the ImageJ wiki).

If it’s a small fix, you can also edit the file directly on github.com. GitHub will automatically fork the repository for you and offer you the possibility to create a pull request.


#3

Great, thank you! I was able to do it directly as it was a small edit.


#4

This is true for collaborators who are “external developers” for that repository. In this case, @gabriel is a “collaborating developer”—actually, the official maintainer in this case—and he does have write access. So @gabriel, a simpler option than the fork-and-PR model is:

  • Clone the repo
  • Make the change (on master branch)
  • Commit the change
  • Push the change (directly to remote master)

It might look something like:

git clone https://github.com/fiji/Auto_Threshold

# make the change
vi src/main/java/fiji/threshold/Auto_Threshold.java

# install new plugin into your Fiji
mvn -Dimagej.app.directory=$HOME/Desktop/Fiji.app -Ddelete.other.versions=true

# run your Fiji to test the change
$HOME/Desktop/Fiji.app/ImageJ-linux64

# push the change
git commit -a -m 'Fix bug in Auto Threshold plugin'
git push

Of course, as @imagejan said, if the change is super simple and can be done directly through the web via the Edit button, that is even simpler.

If the changes are large, you can still make a topic branch and push that, then file a PR. This is useful if you want others to review and discuss the changes before they are merged, and/or if the work is not yet finished.

Once the work is on the master branch, we still need to cut a new Maven release, and then upload that release to the appropriate ImageJ update site, so that users receive the fix. We try to do this every few weeks, or sooner if plugin maintainers request it.

See the Development Lifecycle page for full details on all of this.


#5

Right, I have a few fixes in the auto thresholding plugins.
I tried to replicate what I have done before:

> git clone https://github.com/fiji/Auto_Threshold

Then copied the new sources in the Fiji.app/Auto_Threshold hierarchy, then

> git commit

but I get this message:

 On branch master
 Your branch is up-to-date with 'origin/master'.
 Changes not staged for commit:
         modified:   src/main/java/fiji/threshold/Auto_Local_Threshold.java
         modified:   src/main/java/fiji/threshold/Auto_Threshold.java
 no changes added to commit

Any suggestions?

I use git very little and every time I get stuck with something like this.
I guess that it would be best if the Auto Threshold plugins are removed from the main fiji branch and I create an auto_threshold update site (and include the source in the jar for those who want to play with it).
Thanks for any further comments.


#6

Hello @gabriel,

You need to add the changes before you commit them. Do you use any GUI for git? I use git-gui, which makes all of this process much friendlier.


#7

Right I have gitk and git-gui. What do I do now? :slight_smile:
It shows “Unstaged Changes” somewhere in the Current Branch: Master box
Thanks Ignacio!


#8

Exactly. If you open git-gui, click on the unstaged files until they pass to the box below, then write a commit message and commit :slight_smile:


#9

Wow, thanks. That makes it look easy now. Many thanks. I think I pushed it successful.
Cheers! :thumbsup:


#10

Yes, you did! I’m glad I could help.


#11

Oh dear. I compiled this in plain IJ, which has statements like

import ij.*;

but I see that the difference with the previous file does not seem to support “*”?
Did I do a “no-no”?


#12

Wildcard imports are considered “bad practice”, as they can lead to unintended class name collisions (see this SO post). In essence, code that contains wildcard imports can break simply due to updating a third-party library.

The Fiji and ImageJ2 developers therefore only use explicit imports.


Also see the Coding style page on the wiki for general considerations (formatting your git commit messages etc.).


#13

Thanks for replying.
Strange If I add the original lines that were removed, the plugin does not compile in plain IJ anymore.
To make things worse, I cannot install maven right now as the opensuse repo cannot be added for some reason…
What should I do, revert the fixes I pushed? If so, how do I do this. Thanks.


#14

Well it compiles in Fiji from the script editor. Perhaps I can leave it like that until I resolve al the imports I have to add so it compiles in plain IJ too.


#15

Travis is happy, too: https://travis-ci.org/fiji/Auto_Threshold

Yes, I think you can leave it like that for now.

I just wonder why it wouldn’t compile from ImageJ1.x with the explicit imports. What’s the error message you get?


#16

This was missing in Auto_Threshold:

import ij.process.StackConverter;

Quite odd how did the Fiji version compiled then?
If I reinstate the old imported lines plus that one (above) then it compiles fine.


#17

Maybe because it wasn’t missing before your commit, see this line :wink: ?

As for the script editor, maybe you had the Edit > Auto-import (deprecated) option enabled?


#18

Just pushed the imports without wildcards… I learned a few things today. Thanks for the help.