feedzai / feedzai-openml-java

Implementations for Feedzai's OpenML APIs to allow for usage of machine learning models in the Java programming language.
https://www.feedzai.com
Apache License 2.0
2 stars 11 forks source link

Add workdir on lightgbm make file #32

Closed shengwangsw closed 4 years ago

shengwangsw commented 4 years ago

After set -e we need to define the workdir

codecov[bot] commented 4 years ago

Codecov Report

Merging #32 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #32   +/-   ##
=========================================
  Coverage     77.69%   77.69%           
  Complexity      357      357           
=========================================
  Files            39       39           
  Lines          1318     1318           
  Branches        120      120           
=========================================
  Hits           1024     1024           
  Misses          228      228           
  Partials         66       66           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 99e8c82...3198693. Read the comment docs.

shengwangsw commented 4 years ago

The root cause for this PR has to do with this PR#30 and PR#31.

JPDSousa commented 4 years ago

After set -e we need to define the workdir

@shenggwang it's good practice to elaborate a bit on the problem. I honestly don't know why you need to cd after set -e. You can leave a link here (e.g., a stack overflow question of someone with the same problem) or just elaborate on the problem using your own words.

shengwangsw commented 4 years ago

The problem has to do with the directory, for the reason that I added cd. Because by default it was working on \tmp. Thank you for your question, it makes me realise that it shouldn't be on script itself, instead on pom.xml :smile:

AlbertoEAF commented 4 years ago

The problem has to do with the directory, for the reason that I added cd. Because by default it was working on \tmp. Thank you for your question, it makes me realise that it shouldn't be on script itself, instead on pom.xml

Yes! set -e is only a mode, doesn't affect directories, but the new solution is much better ;)

shengwangsw commented 4 years ago

The problem has to do with the directory, for the reason that I added cd. Because by default it was working on \tmp. Thank you for your question, it makes me realise that it shouldn't be on script itself, instead on pom.xml

Yes! set -e is only a mode, doesn't affect directories, but the new solution is much better ;)

I don't know the reason, by debugging it with pwd, before set -e it is in the correct directory, after that, the directory changed to /tmp. It's weird, I don't know if this have to do with exec-maven-plugin, but I didn't find anything related this issue.