groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul King <pa...@asert.com.au>
Subject Re: GROOVY-7378
Date Mon, 11 May 2015 14:13:20 GMT

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 <jwadamson@gmail.com <mailto:jwadamson@gmail.com>>:
>
>     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


Mime
View raw message