[
https://issues.apache.org/jira/browse/OFBIZ-10458?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16534166#comment-16534166 ]
Gil Portenseigne commented on OFBIZ-10458:
------------------------------------------
Hello Mathieu,
I started to review your patches and see interesting things in it.
First thanks for your work, this made me discover new coding tricks, i like it a lot.
My first feedback :ยท
* Import class with an alias to use it can ease java writing, like in your example UtilValidate as Uv. We just need to be careful to use the same all over the code as a convention to keep consistency.
* You propose to store test classes in a subfolder just under src directory. I think that's a nice idea to separate test code (i remember some discussion about that, but cant figure out yet the details, after some search OFBIZ-4211, but i dont know why this wasnt finished...), this should be discussed in dev mailing list.
* You add failing test, we need to make it pass now, i wondered if we could use groovy testCase "groovy-test-suite" like FileUtilTests.groovy, since you needed to instantiate a GroovyScriptEngine Object ?
* I played a bit and attached a patch containing all your patches, but with some modifications to make your tests pass (quickly moved the test class into main directory (not in a test package for now, it'd be better), add test in front of testing method names, extend OFBizTestCase class ...)
* You let some tabs for indentation, there must be replaced by spaces, no big deal :)
I'll be glad to help you out on these subject :), let me know where you wanna go forward !
Gil
> GetLocaleList call can provide duplicate results
> ------------------------------------------------
>
> Key: OFBIZ-10458
> URL:
https://issues.apache.org/jira/browse/OFBIZ-10458> Project: OFBiz
> Issue Type: Bug
> Reporter: Mathieu Lirzin
> Assignee: Gil Portenseigne
> Priority: Minor
> Fix For: Upcoming Branch
>
> Attachments: OFBIZ-10458_0001-Add-failing-tests.patch, OFBIZ-10458_0002-Fix-duplicates-bug.patch, OFBIZ-10458_0003-Refactor.patch, OFBIZ-10458_with_working_test.patch
>
>
> This is not a huge issue but I detected an issue with the {{GetLocaleList}} script which when providing both a {{localeString}} and {{localeName}} can provide duplicate results.
> Here is a set of 3 patches that are should be applied in order:
> - the first one is adding some tests
> - the second one resolves the bug
> - the third one refactors the code to use a more functional style
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Locked
