search

TheJacksonLaboratory / LIRICAL

LIkelihood Ratio Interpretation of Clinical AbnormaLities
https://thejacksonlaboratory.github.io/LIRICAL/stable
Other
22 stars 11 forks source link

Make release tag and use it in tutorial #506

Closed ielis closed 4 years ago

ielis commented 4 years ago

The part Installation contains a snippet below.

$ git clone https://github.com/TheJacksonLaboratory/LIRICAL.git
$ cd LIRICAL
$ mvn compile
$ mvn package
$ java -jar target/LIRICAL.jar
$ Usage: <main class> [options] [command] [command options]
  Options:
    -h, --help
      display this help message
  (...)

This will build and run the latest version of the code from master. I think we should tag a specific commit with e.g. v1.0.0-RC4 tag and add a line git checkout v1.0.0-RC4 (or similar) to the snippet.

ielis commented 4 years ago

Also, the line mvn compile is redundant in this snippet.

pnrobinson commented 4 years ago

@ielis -- We will shortly be releasing version 1.0.0 -- this was the last RC. I seem to recall that it was necessary to first do mvn compile because of the protobuf. Just doing mvn build resulted in an error. Can you confirm you were trying from a fresh build?

ielis commented 4 years ago

As far as I know, mvn compile is necessary if you want to run the app from IntelliJ as an Application. From some reason, *.proto files are not compiled before running the app in IntelliJ and therefore it fails unless you run mvn compile beforehead.

It is weird that it this is an issue in Lirical, we do not use any *.proto files AFAIK and the generated sources/ protobuf classes from Exomiser should already be present in Exomiser JARs.

Maven lifecycle reference states in section Using Command Line Calls , that mvn install will implicitly run mvn validate, compile, test, package, etc.. So if you have mvn compile; mvn package in the snippet, the first command is redundant. And yes, I was trying from a fresh build - mvn clean package worked.

pnrobinson commented 4 years ago

revised the docs! I am less convinced about tagging the release -- the tutorial should work for all versions, and if there is a change in the API we should change the online tutorial. We should tag the final version of the code that is used for the published analysis. I am guessing this will be v.1.0.1 because we may change some things at the revision phase. I will release v.1.0.0 by Monday.