Hi Robert, Thanks for the response.
The patch for MEXEC-121 already checks whether System.console() exists and falls back to System.out if it does not (i.e. on JDK5). I looked at the test harness (invoker). I'm not sure I can use it to test MEXEC-121, since the test harness captures the output, and so System.console() will return null and the patch will fall back to using System.out in this case. Even if the test harness doesn't capture the output, and the patch uses System.console() to write directly to the console, that won't help testing, b/c then there's no way to validate what is written to the console... MEXEC-118 on the other hand looks very testable, only it's a Windows specific feature, and I assume the release builds are done on Linux? In any case, I will write a test for it and put it under a profile activated by <family>windows</family>. Thanks, Trask On Sat, Jan 4, 2014 at 3:08 PM, Robert Scholte <codeh...@sourcegrounds.com>wrote: > Hi, > > thanks for the patches. > A few remarks: Even though JDK6 has reached EOL, Mavens requirement is > still JDK5. > So for MEXEC-121 you need to check if java.io.Console is available, since > it was introduced with Java6 > > In both cases it would be nice to add an integration test, based on the > maven-invoker-plugin[1], to confirm your solution. > There are already several tests under src/it > Run 'mvn verify' to confirm that everything still works. > > thanks, > Robert > > [1] http://maven.apache.org/plugins/maven-invoker-plugin/ > > > Op Sat, 04 Jan 2014 23:54:59 +0100 schreef Trask Stalnaker < > trask.stalna...@gmail.com>: > > > Hi, >> >> I have submitted 2 patches to the maven-exec-plugin project. What's the >> best way to help move these forward? >> >> http://jira.codehaus.org/browse/MEXEC-118 >> >> http://jira.codehaus.org/browse/MEXEC-121 >> >> Thanks, >> Trask >> > > --------------------------------------------------------------------- > To unsubscribe from this list, please visit: > > http://xircles.codehaus.org/manage_email > > >
