You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "dion gillard (JIRA)" <ji...@apache.org> on 2007/10/28 07:30:50 UTC
[jira] Created: (JEXL-35) Final API requirements
Final API requirements
----------------------
Key: JEXL-35
URL: https://issues.apache.org/jira/browse/JEXL-35
Project: Commons JEXL
Issue Type: Improvement
Affects Versions: 2.0
Reporter: dion gillard
Fix For: 2.0
Which classes should be public?
Is the package structure reasonable?
Are classes in the right packages?
Is it easily extensible?
...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro updated JEXL-35:
------------------------------
Attachment: JEXL-35.patch
Thanks for the heads up Rahul.
Continued the clean up:
util.Introspector, MethodCache, MethodMap & util.introspection.Introspector are "locked down" for derivation (ie private or final wherever possible).
Updated to newest version of maven-javacc plugin.
Renamed JEXLNode to JexlNode (naming convention).
Some refactoring around Info to try to get better debug information.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch, JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro updated JEXL-35:
------------------------------
Attachment: JEXL-35.patch
Added missing 2 * package.html
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch, JEXL-35.patch, JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12735593#action_12735593 ]
Henri Biestro edited comment on JEXL-35 at 7/27/09 5:52 AM:
------------------------------------------------------------
The whole refactoring has moved to JEXL-60 .
I've removed all the previous patches/attachements to avoid confusion.
This JEXL-35 issue still stands but is pretty much adressed through JEXL-60.
was (Author: henrib):
The whole refactoring has moved to JEXL-60 .
This JEXL-35 issue still stands but is pretty much adressed through JEXL-60.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro updated JEXL-35:
------------------------------
Attachment: (was: JEXL-35.patch)
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JEXL-35) Final API requirements
Posted by "Rahul Akolkar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734122#action_12734122 ]
Rahul Akolkar commented on JEXL-35:
-----------------------------------
Heads up that I'm traveling this week and won't be able to look at any JEXL patches till am back (so, looking at mid next week time frame unless someone else volunteers here).
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro updated JEXL-35:
------------------------------
Attachment: (was: JEXL-35.patch)
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch, JEXL-35.patch, JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro updated JEXL-35:
------------------------------
Attachment: (was: JEXL-35.patch)
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch, JEXL-35.patch, JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12735593#action_12735593 ]
Henri Biestro commented on JEXL-35:
-----------------------------------
The whole refactoring has moved to JEXL-60 .
This JEXL-35 issue still stands but is pretty much adressed through JEXL-60.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro updated JEXL-35:
------------------------------
Attachment: JEXL-35.patch
Cleaner, less convoluted refactoring.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch, JEXL-35.patch, JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro updated JEXL-35:
------------------------------
Attachment: JEXL-35.patch
I went ahead and refactored the o.a.c.jexl.util & o.a.c.jexl.util.introspection.
The idea was to keep the packages & keep sense; thus, the util.Introspector has been beefed up with "high level" property set/get per-type and the Uberspect deals with finding the most appropriate executors.
It's probably a tad slower than the previous code (what used to be in the Interpreter) but "clearer" in what is performed (imho of course :-) ).
Let me know if I should continue on this route or if this is really too big of a change.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12735252#action_12735252 ]
Henri Biestro edited comment on JEXL-35 at 7/25/09 3:44 AM:
------------------------------------------------------------
Added missing 2 * package.html.
Some files need to be removed (svn del):
src/java/org/apache/commons/jexl/Arithmetic.java
src/java/org/apache/commons/jexl/parser/JEXLNode.java (*)
src/java/org/apache/commons/jexl/util/PropertyExecutor.java
src/java/org/apache/commons/jexl/util/BooleanPropertyExecutor.java
src/java/org/apache/commons/jexl/util/introspection/UberspectLoggable.java
src/java/org/apache/commons/jexl/util/GetExecutor.java
If on windows, you really need to remove it before applying the patch otherwise it will just keep using JEXLNode.java for a class named JexlNode...
The patch does not apply cleanly especially UberspectImpl & Introspector.
The "best" approach I've found yet is to force apply updates even if patch thinks it may have already been applied.
Then, for each of them, copy/paste the '.rej' content replacing the end of the original file with the content at the proper place; a quick regexp replace with '^+ ' to get rid of these '+' and it should be ok.
I wish I had a way to generate patches guaranteed to apply (a tool that would apply the patch to the original file & if rejects are found, re-create a patch for this file that replaces the whole content - delete all original content, add all new content - with the new one....)
was (Author: henrib):
Added missing 2 * package.html
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch, JEXL-35.patch, JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733954#action_12733954 ]
Henri Biestro edited comment on JEXL-35 at 7/21/09 7:34 PM:
------------------------------------------------------------
I went ahead and refactored the o.a.c.jexl.util & o.a.c.jexl.util.introspection. (removed some classes that were not used and moved the usefull code in util.introspector; added test to improve coverage).
The idea was to keep the packages & keep sense; thus, the util.Introspector has been beefed up with "high level" property set/get per-type and the Uberspect deals with finding the most appropriate executors.
It's probably a tad slower than the previous code (what used to be in the Interpreter) but "clearer" in what is performed (imho of course :-) ).
Let me know if I should continue on this route or if this is really too big of a change (I will still have to go redo a bit of JEXL-20).
was (Author: henrib):
I went ahead and refactored the o.a.c.jexl.util & o.a.c.jexl.util.introspection.
The idea was to keep the packages & keep sense; thus, the util.Introspector has been beefed up with "high level" property set/get per-type and the Uberspect deals with finding the most appropriate executors.
It's probably a tad slower than the previous code (what used to be in the Interpreter) but "clearer" in what is performed (imho of course :-) ).
Let me know if I should continue on this route or if this is really too big of a change.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12732709#action_12732709 ]
Henri Biestro commented on JEXL-35:
-----------------------------------
Package structure looks good & clear;
It seems extensibility & configurability are covered & trying to reduce visibility of some classes would reduce what we gained on these aspects.
The last "may be" is whether we should keep/get rid of Arithmetic and make JexlArithmetic public (adding a toString() to it for completeness); and other methods as protected; there does not seem to be much value to Arithmetic as an interface by itself imho.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro updated JEXL-35:
------------------------------
Attachment: JEXL-35.patch
More (pure) Checkstyle fixes.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch, JEXL-35.patch, JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro resolved JEXL-35.
-------------------------------
Resolution: Fixed
Assignee: Henri Biestro
If there are any remaining API issues, they should get their own issue #.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Assignee: Henri Biestro
> Fix For: 2.0
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henri Biestro updated JEXL-35:
------------------------------
Attachment: JEXL-35.patch
I need to move this to a different bug soon to be created as 'refactoring jexl.util'...
Completed the classes in jexl.util and added tests (coverage much improved).
Added a per-ast-node cache of executors which speeds up evaluation
Added more debug functionality so it is possible to track the origin of errors (either code or file)
Checkstyle itself is 'ok' I guess (Checkstyle prefers inheritDoc to @Override but the later is much more helpfull in an IDE).
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
> Attachments: JEXL-35.patch, JEXL-35.patch, JEXL-35.patch, JEXL-35.patch
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JEXL-35) Final API requirements
Posted by "Rahul Akolkar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12732837#action_12732837 ]
Rahul Akolkar commented on JEXL-35:
-----------------------------------
Theres value in being able to supply your own Arithmetic, but as an interface its probably more of an encumbrance for code evolution (I can imagine other methods being added over time) so I tend to agree.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JEXL-35) Final API requirements
Posted by "Henri Biestro (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733179#action_12733179 ]
Henri Biestro commented on JEXL-35:
-----------------------------------
I think the Ubespect also needs to be reworked; it should be the class to derive to set/get properties & execute methods. Besides, the original Velociy ties dont really bring value here and some of the code in UberspectImpl is not really as clear as I'd personally like. And there is some broken simmetry between get & set ("gettable" vs map) in method discovery.
As of packaging goes, it seems a jexl.introspection package should be the place to put all these in; however, it might not be possible if we are to maintain compatiblity at this level.
I'll submit a patch soon without the re-packaging to have a better view.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (JEXL-35) Final API requirements
Posted by "Rahul Akolkar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JEXL-35?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733375#action_12733375 ]
Rahul Akolkar commented on JEXL-35:
-----------------------------------
Yup, lets try without the repackaging. BTW, the introspection package has the worst test coverage of the lot, would be good if that were a couple of notches higher before going to v2.0 RC.
> Final API requirements
> ----------------------
>
> Key: JEXL-35
> URL: https://issues.apache.org/jira/browse/JEXL-35
> Project: Commons JEXL
> Issue Type: Improvement
> Affects Versions: 2.0
> Reporter: dion gillard
> Fix For: 2.0
>
>
> Which classes should be public?
> Is the package structure reasonable?
> Are classes in the right packages?
> Is it easily extensible?
> ...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.