[
https://issues.apache.org/jira/browse/OFBIZ-11926?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17742991#comment-17742991
]
Paul Foxworthy edited comment on OFBIZ-11926 at 7/14/23 5:32 AM:
-----------------------------------------------------------------
For the record, this change introduced a bug.
See the changes to SQLProcessor.java at line 80 in
[https://github.com/apache/ofbiz-framework/commit/f357b997c0ba2d1459055088d962a5168eee5c4d#]
The field _sql was renamed to sql. Trouble is, there was a local variable also
named sql in one of the overloaded prepareStatement methods, so the result was
a no-op
{{sql = sql;}}
at
[https://github.com/apache/ofbiz-framework/blob/dbaed189b17b34da9133a52f569e09e2e07ae6f9/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/SQLProcessor.java#L376]
The line should read
{{this.sql = sql;}}
OFBIZ-12386 then deleted the no-op, see
[https://github.com/apache/ofbiz-framework/commit/997cf16ed317a92dea18e2b661a366c72f4bf5ae#diff-8d94a7797abfc769581da44dcc951e6a51fba95678a5bfd851a9aa44b13ea63e]
The fix is in OFBIZ-12837.
I grepped through all the changes in OFBIZ-11926 and couldn't find any other
incidence of a "fred = fred" line, so I think this was the only problem.
I'd like to suggest some things that might avoid similar problems in future.
I suspect the changes in OFBIZ-11926 were done with a dumb text
search-and-replace. A refactoring tool would given a warning that the field
would be hidden by a local variable of the same name. So I suggest we use
refactoring tools for style improvements.
The commit made 1,714 additions and 988 deletions over 88 files. That's a lot
to review. Can we keep commits smaller than that?
was (Author: paul_foxworthy):
For the record, this change introduced a bug.
See the changes to SQLProcessor.java at line 80 in
[https://github.com/apache/ofbiz-framework/commit/f357b997c0ba2d1459055088d962a5168eee5c4d#]
The field _sql was renamed to sql. Trouble is, there was a local variable also
named sql in one of the overloaded prepareStatement methods, so the result was
a no-op
{{sql = sql;}}
at
[https://github.com/apache/ofbiz-framework/blob/dbaed189b17b34da9133a52f569e09e2e07ae6f9/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/SQLProcessor.java#L376]
The line should read
{{this.sql = sql;}}
OFBIZ-12386 then deleted the no-op, see
[https://github.com/apache/ofbiz-framework/commit/997cf16ed317a92dea18e2b661a366c72f4bf5ae#diff-8d94a7797abfc769581da44dcc951e6a51fba95678a5bfd851a9aa44b13ea63e]
I will post another Jira for the fix.
I grepped through all the changes in OFBIZ-11926 and couldn't find any other
incidence of a "fred = fred" line, so I think this was the only problem.
I'd like to suggest some things that might avoid similar problems in future.
I suspect the changes in OFBIZ-11926 were done with a dumb text
search-and-replace. A refactoring tool would given a warning that the field
would be hidden by a local variable of the same name. So I suggest we use
refactoring tools for style improvements.
The commit made 1,714 additions and 988 deletions over 88 files. That's a lot
to review. Can we keep commits smaller than that?
> Checkstyle: Variable name must match pattern
> --------------------------------------------
>
> Key: OFBIZ-11926
> URL: https://issues.apache.org/jira/browse/OFBIZ-11926
> Project: OFBiz
> Issue Type: Sub-task
> Components: ALL COMPONENTS
> Affects Versions: Trunk
> Reporter: Suraj Khurana
> Assignee: Suraj Khurana
> Priority: Major
> Attachments: JsLanguageFilesMapping.patch, OFBIZ-11926-plugins.patch,
> OFBIZ-11926.patch
>
>
> All final data members of the class must match this naming pattern
> '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
