[GitHub] ant pull request #30: Patch compile script

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #30: Patch compile script

bodewig
GitHub user pyxide opened a pull request:

    https://github.com/apache/ant/pull/30

    Patch compile script

   

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/pyxide/ant PATCH_COMPILE_SCRIPT

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/ant/pull/30.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #30
   
----
commit 824a17b2b78615da01433b333ca4861b8402f31a
Author: pyxide <[hidden email]>
Date:   2017-01-13T16:16:50Z

    Scriptdef task new options : 'compiled' (if javax.script engine
    implements Compilable), 'encoding' to load resources

commit ea81a70738db8b3a5f5ab8f3199fb34201560b31
Author: pyxide <[hidden email]>
Date:   2017-01-16T12:48:26Z

    Fix CRLF to LF

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #30: Enhance scriptdef with a 'compiled' option

bodewig
Github user bodewig commented on the issue:

    https://github.com/apache/ant/pull/30
 
    Many thanks, this looks good,
   
    While looking through your changes I wonder whether we couldn't remove part of the reflection logic for master as we target Java8 and `javax.script` should be there.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #30: Enhance scriptdef with a 'compiled' option

bodewig
In reply to this post by bodewig
Github user pyxide commented on the issue:

    https://github.com/apache/ant/pull/30
 
    Thank you for your positive review.
   
    > remove part of the reflection logic for master as we target Java8 and javax.script should be there
   
    I really don't have a strong opinion on this point. I did the first prototype with `javax.script` classes but used reflection to accommodate to the rest of the code.
   
    While adding some test units, I discovered that the BSF manager may automatically compiles scripts.
    Had the bsf library been on my Ant classpath before, I wouldn't have developped this feature.
    Should we compile the scripts by default ?
   
    On another note, I added the `encoding` option on the other script components that use
    the `src` attribute (ScriptCondition, ScriptMapper, ScriptFilter, ScriptSelector).



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #30: Enhance scriptdef with a 'compiled' option

bodewig
In reply to this post by bodewig
Github user bodewig commented on the issue:

    https://github.com/apache/ant/pull/30
 
    WRT reflection, let's keep it the way it is for now.
   
    I'm not a user if the `script` family so I'm not sure what the default should be. It probably costs extra time, do I benefit from compilation if the script is run exactly once?
   
    `encoding` is a good idea, thank you.
   
    > I guess something along the line of MagicNames.SCRIPT_REPOSITORY = org.apache.ant.scriptrepo would be better.
   
    +1



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #30: Enhance scriptdef with a 'compiled' option

bodewig
In reply to this post by bodewig
Github user bodewig commented on the issue:

    https://github.com/apache/ant/pull/30
 
    @pyxide are you going to going to change this PR to use a `MagicNames` constant?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #30: Enhance scriptdef with a 'compiled' option

bodewig
In reply to this post by bodewig
Github user pyxide commented on the issue:

    https://github.com/apache/ant/pull/30
 
    @bodewig, sorry for delay, the MagicNames change has now landed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant issue #30: Enhance scriptdef with a 'compiled' option

bodewig
In reply to this post by bodewig
Github user bodewig commented on the issue:

    https://github.com/apache/ant/pull/30
 
    no worries, I wasn't sure whether you still wanted to change the code or leave it for a later fix.
   
    Many thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] ant pull request #30: Enhance scriptdef with a 'compiled' option

bodewig
In reply to this post by bodewig
Github user asfgit closed the pull request at:

    https://github.com/apache/ant/pull/30


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]