compomics / peptide-shaker

Interpretation of proteomics identification results
http://compomics.github.io/projects/peptide-shaker.html
48 stars 18 forks source link

GIT tag (or commit ID) on releases #211

Open max-l opened 8 years ago

max-l commented 8 years ago

I am currently looking at a bug that was reported fixed in release 1.13.4, by a pull request I have submitted. By looking at the stack trace in the logs, It seems like the pull request might have not made it in this release. The only way that I can be certain would be to decompile the jar.

This maven plugin would solve the problem: https://github.com/ktoso/maven-git-commit-id-plugin

I haven't used it, and perhaps it's too complicated for nothing, if it's the case, a simple script that gets called during your build process and adds a file to the jar would do the trick

an even cheaper solution (but less automated, so more error prone) would be to just publish the commit ID along with the release notes, but again, the more manual steps, the more risk of errors.

hbarsnes commented 8 years ago

I assume the pull request your referring to is this one: https://github.com/compomics/peptide-shaker/pull/207? As you can see it was accepted and merged into the master branch. And the release are always done from the current version of the master branch at the time of the release. I've also checked the code and the two lines you changed are still there. Perhaps the process exists at a different point?

Regarding adding a GIT tag or commit ID to each release, it all sounds a bit too complicated given the way we release new versions? Could you not get what you need simply by checking the current master version at the time point when the new release was made?

I do however see the advantage of including this information in the release, so I will try to get the time to look into it at some point. No guarantees as to when this may happen though.

max-l commented 8 years ago

The #207 I saw had the exact same stack trace, so in theory it should have been caught by the "catch" clause at line 361... That's in theory, perhaps something is escaping me.

It's true that with a release date I can deduct the state of master at the release time, I just like the idea of removing any manual step that can cause errors. Personally I like to do things like put the commit ID in exception dumps, so that for any exception reported, one can build the exact jar version under which it occurred. Now that I think of it, dumping the commit ID in the exception dumps might be simpler (you don't have to deal with overly complicated maven plugin.... just the thought of doing anything with maven depresses me ;-))

But I also totally understand if such things are not high on your priority list, it's just a suggestion...

Keep up the good work, peptide-shaker is extremely helpful to us !

hbarsnes commented 8 years ago

The #207 I saw had the exact same stack trace, so in theory it should have been caught by the "catch" clause at line 361... That's in theory, perhaps something is escaping me.

But even if the exception is caught the stacktrace is still printed. So not sure how the exit value would affect this?

Personally I like to do things like put the commit ID in exception dumps, so that for any exception reported, one can build the exact jar version under which it occurred. Now that I think of it, dumping the commit ID in the exception dumps might be simpler (you don't have to deal with overly complicated maven plugin.... just the thought of doing anything with maven depresses me ;-))

Sounds reasonable. How do I add this information to the exception dumps then? Are you already doing this yourself?

max-l commented 8 years ago

But even if the exception is caught the stacktrace is still printed. So not sure how the exit value would affect this?

The exit value is only for notifying the calling process that something went wrong, to prevent the next program in the pipeline from starting with invalid (or non existent) input. We call peptide-sharer thousands of times from some kind of "pipeline manager" so knowing which prog in the pipeline crashed is valuable.

Sounds reasonable. How do I add this information to the exception dumps then? Are you already doing this yourself?

That requires that the dumping/printing of exception in the project is handled by a single function (i.e. a static main like : MyProject.dumpException(ex)), this functions then has the commitID (ex: from a config file, or even hardcoded...)