-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: provide sensible default values for RtScanner #837
Conversation
35c285d
to
926a25d
Compare
@btellstrom the CI failure seems to be unrelated to this change: https://travis-ci.org/Spirals-Team/repairnator/jobs/521297233 Since it's related to dates, it may be a flaky test. WDYT? |
ready for review. the goal is to be able to run the Scanner only as fast as possible (I spent hours debugging the parameters one after the other, finding out that they were not necessary in a default run). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good to me, just the default value that might need to be added.
@@ -332,7 +326,6 @@ public static FlaggedOption defineArgDockerImageName() { | |||
opt.setShortFlag('n'); | |||
opt.setLongFlag("name"); | |||
opt.setStringParser(JSAP.STRING_PARSER); | |||
opt.setRequired(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the required
flag is removed we should probably set a default of spirals/repairnator:latest
.
opt.setStringParser(fileStringParser); | ||
|
||
if (launcherType == LauncherType.DOCKERPOOL || launcherType == LauncherType.REALTIME || launcherType == LauncherType.CHECKBRANCHES) { | ||
opt.setDefault("/tmp/repairnator-output"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure exactly what this change would do, today when we run the RTScanner logs are created in the $HOME
directory of the user running the RTScanner, and output is not to a single file, but rather there is log files for each container run as well as for the RTScanner itself. As I understand these changes, they would not allow this behaviour?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realised this might be mitigated by the changes below, but I will keep this comment for now.
Just realised that the |
@btellstrom merge? |
so as to run RtScanner without having to read the documentation