[jira] [Commented] (WW-5279) Improve readability of XmlConfigurationProvider class
[ https://issues.apache.org/jira/browse/WW-5279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17682869#comment-17682869 ] ASF subversion and git services commented on WW-5279: - Commit 2f7a50935bab9384df5da6d6270838646e70d635 in struts's branch refs/heads/master from Kusal Kithul-Godage [ https://gitbox.apache.org/repos/asf?p=struts.git;h=2f7a50935 ] WW-5279 Improve readability of XmlConfigurationProvider class > Improve readability of XmlConfigurationProvider class > - > > Key: WW-5279 > URL: https://issues.apache.org/jira/browse/WW-5279 > Project: Struts 2 > Issue Type: Task > Components: Core >Reporter: Kusal Kithul-Godage >Priority: Trivial > Fix For: 6.2.0 > > Time Spent: 1h 20m > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Resolved] (WW-5279) Improve readability of XmlConfigurationProvider class
[ https://issues.apache.org/jira/browse/WW-5279?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Lukasz Lenart resolved WW-5279. --- Resolution: Fixed > Improve readability of XmlConfigurationProvider class > - > > Key: WW-5279 > URL: https://issues.apache.org/jira/browse/WW-5279 > Project: Struts 2 > Issue Type: Task > Components: Core >Reporter: Kusal Kithul-Godage >Priority: Trivial > Fix For: 6.2.0 > > Time Spent: 1h 20m > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (WW-5279) Improve readability of XmlConfigurationProvider class
[ https://issues.apache.org/jira/browse/WW-5279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17682870#comment-17682870 ] ASF subversion and git services commented on WW-5279: - Commit 930c6de8076d6ed6037c8d791a5ad7eae69765d5 in struts's branch refs/heads/master from Lukasz Lenart [ https://gitbox.apache.org/repos/asf?p=struts.git;h=930c6de80 ] Merge pull request #657 from atlassian/WW-5279-xml-config-provider WW-5279 Improve readability of XmlConfigurationProvider class > Improve readability of XmlConfigurationProvider class > - > > Key: WW-5279 > URL: https://issues.apache.org/jira/browse/WW-5279 > Project: Struts 2 > Issue Type: Task > Components: Core >Reporter: Kusal Kithul-Godage >Priority: Trivial > Fix For: 6.2.0 > > Time Spent: 1h 20m > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (WW-5279) Improve readability of XmlConfigurationProvider class
[ https://issues.apache.org/jira/browse/WW-5279?focusedWorklogId=842756=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-842756 ] ASF GitHub Bot logged work on WW-5279: -- Author: ASF GitHub Bot Created on: 01/Feb/23 06:23 Start Date: 01/Feb/23 06:23 Worklog Time Spent: 10m Work Description: lukaszlenart merged PR #657: URL: https://github.com/apache/struts/pull/657 Issue Time Tracking --- Worklog Id: (was: 842756) Time Spent: 1h 20m (was: 1h 10m) > Improve readability of XmlConfigurationProvider class > - > > Key: WW-5279 > URL: https://issues.apache.org/jira/browse/WW-5279 > Project: Struts 2 > Issue Type: Task > Components: Core >Reporter: Kusal Kithul-Godage >Priority: Trivial > Fix For: 6.2.0 > > Time Spent: 1h 20m > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (WW-5279) Improve readability of XmlConfigurationProvider class
[
https://issues.apache.org/jira/browse/WW-5279?focusedWorklogId=842581=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-842581
]
ASF GitHub Bot logged work on WW-5279:
--
Author: ASF GitHub Bot
Created on: 31/Jan/23 13:31
Start Date: 31/Jan/23 13:31
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on code in PR #657:
URL: https://github.com/apache/struts/pull/657#discussion_r1091929393
##
core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java:
##
@@ -439,28 +445,16 @@ protected void addAction(Element actionElement,
PackageConfig.Builder packageCon
String name = actionElement.getAttribute("name");
String className = actionElement.getAttribute("class");
//methodName should be null if it's not set
-String methodName =
StringUtils.trimToNull(actionElement.getAttribute("method"));
+String methodName = trimToNull(actionElement.getAttribute("method"));
Location location = DomHelper.getLocationObject(actionElement);
if (location == null) {
LOG.warn("Location null for {}", className);
}
-// if there isn't a class name specified for an then try to
-// use the default-class-ref from the
-if (StringUtils.isEmpty(className)) {
-// if there is a package default-class-ref use that, otherwise use
action support
- /* if (StringUtils.isNotEmpty(packageContext.getDefaultClassRef()))
{
-className = packageContext.getDefaultClassRef();
-} else {
-className = ActionSupport.class.getName();
-}*/
-
-} else {
-if (!verifyAction(className, name, location)) {
-LOG.error("Unable to verify action [{}] with class [{}], from
[{}]", name, className, location);
-return;
-}
+if (!className.isEmpty() && !verifyAction(className, name, location)) {
Review Comment:
@sepe81 I'm right now creating a task to mark a given code `@Deprecated` and
then another task (targeting major/minor release) to remove the code. I found
such approach more useful and more informative for the users :)
Issue Time Tracking
---
Worklog Id: (was: 842581)
Time Spent: 1h 10m (was: 1h)
> Improve readability of XmlConfigurationProvider class
> -
>
> Key: WW-5279
> URL: https://issues.apache.org/jira/browse/WW-5279
> Project: Struts 2
> Issue Type: Task
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Trivial
> Fix For: 6.2.0
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [struts] lukaszlenart commented on a diff in pull request #657: WW-5279 Improve readability of XmlConfigurationProvider class
lukaszlenart commented on code in PR #657:
URL: https://github.com/apache/struts/pull/657#discussion_r1091929393
##
core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java:
##
@@ -439,28 +445,16 @@ protected void addAction(Element actionElement,
PackageConfig.Builder packageCon
String name = actionElement.getAttribute("name");
String className = actionElement.getAttribute("class");
//methodName should be null if it's not set
-String methodName =
StringUtils.trimToNull(actionElement.getAttribute("method"));
+String methodName = trimToNull(actionElement.getAttribute("method"));
Location location = DomHelper.getLocationObject(actionElement);
if (location == null) {
LOG.warn("Location null for {}", className);
}
-// if there isn't a class name specified for an then try to
-// use the default-class-ref from the
-if (StringUtils.isEmpty(className)) {
-// if there is a package default-class-ref use that, otherwise use
action support
- /* if (StringUtils.isNotEmpty(packageContext.getDefaultClassRef()))
{
-className = packageContext.getDefaultClassRef();
-} else {
-className = ActionSupport.class.getName();
-}*/
-
-} else {
-if (!verifyAction(className, name, location)) {
-LOG.error("Unable to verify action [{}] with class [{}], from
[{}]", name, className, location);
-return;
-}
+if (!className.isEmpty() && !verifyAction(className, name, location)) {
Review Comment:
@sepe81 I'm right now creating a task to mark a given code `@Deprecated` and
then another task (targeting major/minor release) to remove the code. I found
such approach more useful and more informative for the users :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@struts.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Work logged] (WW-5279) Improve readability of XmlConfigurationProvider class
[
https://issues.apache.org/jira/browse/WW-5279?focusedWorklogId=842580=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-842580
]
ASF GitHub Bot logged work on WW-5279:
--
Author: ASF GitHub Bot
Created on: 31/Jan/23 13:27
Start Date: 31/Jan/23 13:27
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on code in PR #657:
URL: https://github.com/apache/struts/pull/657#discussion_r1091924829
##
core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java:
##
@@ -164,13 +174,10 @@ public boolean equals(Object o) {
if (this == o) {
return true;
}
-
if (!(o instanceof XmlConfigurationProvider)) {
return false;
}
-
-final XmlConfigurationProvider xmlConfigurationProvider =
(XmlConfigurationProvider) o;
-
+XmlConfigurationProvider xmlConfigurationProvider =
(XmlConfigurationProvider) o;
Review Comment:
I'm fine with dropping `final` in this context, I like using `final`s for
fields and function arguments, where it makes sense :)
Issue Time Tracking
---
Worklog Id: (was: 842580)
Time Spent: 1h (was: 50m)
> Improve readability of XmlConfigurationProvider class
> -
>
> Key: WW-5279
> URL: https://issues.apache.org/jira/browse/WW-5279
> Project: Struts 2
> Issue Type: Task
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Trivial
> Fix For: 6.2.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [struts] lukaszlenart commented on a diff in pull request #657: WW-5279 Improve readability of XmlConfigurationProvider class
lukaszlenart commented on code in PR #657:
URL: https://github.com/apache/struts/pull/657#discussion_r1091924829
##
core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java:
##
@@ -164,13 +174,10 @@ public boolean equals(Object o) {
if (this == o) {
return true;
}
-
if (!(o instanceof XmlConfigurationProvider)) {
return false;
}
-
-final XmlConfigurationProvider xmlConfigurationProvider =
(XmlConfigurationProvider) o;
-
+XmlConfigurationProvider xmlConfigurationProvider =
(XmlConfigurationProvider) o;
Review Comment:
I'm fine with dropping `final` in this context, I like using `final`s for
fields and function arguments, where it makes sense :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@struts.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Work logged] (WW-5279) Improve readability of XmlConfigurationProvider class
[
https://issues.apache.org/jira/browse/WW-5279?focusedWorklogId=842561=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-842561
]
ASF GitHub Bot logged work on WW-5279:
--
Author: ASF GitHub Bot
Created on: 31/Jan/23 12:20
Start Date: 31/Jan/23 12:20
Worklog Time Spent: 10m
Work Description: sepe81 commented on code in PR #657:
URL: https://github.com/apache/struts/pull/657#discussion_r1091847537
##
core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java:
##
@@ -164,13 +174,10 @@ public boolean equals(Object o) {
if (this == o) {
return true;
}
-
if (!(o instanceof XmlConfigurationProvider)) {
return false;
}
-
-final XmlConfigurationProvider xmlConfigurationProvider =
(XmlConfigurationProvider) o;
-
+XmlConfigurationProvider xmlConfigurationProvider =
(XmlConfigurationProvider) o;
Review Comment:
https://stackoverflow.com/questions/154314/when-should-one-use-final-for-method-parameters-and-local-variables/154510#154510
LGTM, but for this existing line "If I'm in someone else's code, I'm not going
to pull them out but if I'm writing new code I won't put them in." would fit.
With your explanation, both styles would be ok for me. Maybe @lukaszlenart
has some recommendation according to the Struts coding style?
Issue Time Tracking
---
Worklog Id: (was: 842561)
Time Spent: 50m (was: 40m)
> Improve readability of XmlConfigurationProvider class
> -
>
> Key: WW-5279
> URL: https://issues.apache.org/jira/browse/WW-5279
> Project: Struts 2
> Issue Type: Task
> Components: Core
>Reporter: Kusal Kithul-Godage
>Priority: Trivial
> Fix For: 6.2.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
[GitHub] [struts] sepe81 commented on a diff in pull request #657: WW-5279 Improve readability of XmlConfigurationProvider class
sepe81 commented on code in PR #657:
URL: https://github.com/apache/struts/pull/657#discussion_r1091847537
##
core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java:
##
@@ -164,13 +174,10 @@ public boolean equals(Object o) {
if (this == o) {
return true;
}
-
if (!(o instanceof XmlConfigurationProvider)) {
return false;
}
-
-final XmlConfigurationProvider xmlConfigurationProvider =
(XmlConfigurationProvider) o;
-
+XmlConfigurationProvider xmlConfigurationProvider =
(XmlConfigurationProvider) o;
Review Comment:
https://stackoverflow.com/questions/154314/when-should-one-use-final-for-method-parameters-and-local-variables/154510#154510
LGTM, but for this existing line "If I'm in someone else's code, I'm not going
to pull them out but if I'm writing new code I won't put them in." would fit.
With your explanation, both styles would be ok for me. Maybe @lukaszlenart
has some recommendation according to the Struts coding style?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@struts.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
