Yes, ideally we'd like to check a few combinations of the different shells the script attempts to handle, e.g. linux, Mac, cygwin, msys, etc. There are a few places earlier on in the script which set GROOVY_HOME, JAVA_HOME, etc. It would be nice to check whether additional quote escaping wasn't needed in the earlier references to the variables you're escaping. Ideally we'd like a CI environment with access to multiple shells and some integration tests. We don't yet have that. In the interim all I can suggest is to add a few scenarios that you've tested against to the PR description (or as a comment or into the JIRA issue). Then perhaps other people can test the same scenarios across some other environments.
Cheers, Paul. On 11/05/2015 11:48 PM, Cédric Champeau wrote:
Hi Jeff Sorry for not commenting on the PR yet. Basically we're very conservative regarding changes to run scripts, because it's easy to break things. We don't have any integration test that would make sure we don't break something, so we'll have to check what your PR implies in terms of compatibility. 2015-05-11 15:33 GMT+02:00 Jeff Adamson <[email protected] <mailto:[email protected]>>: I filed a bug along with a patch and github pull request a few weeks back. This was around the time of the migration to apache, but seems to have everything in the correct place. https://issues.apache.org/jira/browse/GROOVY-7378 https://github.com/apache/incubator-groovy/pull/5 I see other pull requests that have been merged and closed. What is the best way to get feedback on my patch and/or merge it in? --Jeff
--- This email has been checked for viruses by Avast antivirus software. http://www.avast.com
