You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2020/07/03 07:31:31 UTC
[GitHub] [tomcat] martin-g opened a new pull request #312: Use StringBuilder instead of StringBuffer
martin-g opened a new pull request #312:
URL: https://github.com/apache/tomcat/pull/312
There is no need of synchronization when it is a method local variable.
Append character instead of String when possible
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on pull request #312: Use StringBuilder instead of StringBuffer
Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #312:
URL: https://github.com/apache/tomcat/pull/312#issuecomment-653579373
Looks good to me now. The change for rewrite is likely the most significant, the rest is probably not doing anything though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] pzygielo commented on pull request #312: Use StringBuilder instead of StringBuffer
Posted by GitBox <gi...@apache.org>.
pzygielo commented on pull request #312:
URL: https://github.com/apache/tomcat/pull/312#issuecomment-653434713
> and which `append(char)` might be wrong.
I suppose, there is change in produced result
```
(1) retval.append("\\t");
(2) retval.append('\t');
```
as in first case "\t" (i.e. two-char string: `\` followed by `t`) is appended, and in second case - tab character ('\t') is.
Unfortunately I can't tell if there is test to validate what is expected as all smoke test jobs pass.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on pull request #312: Use StringBuilder instead of StringBuffer
Posted by GitBox <gi...@apache.org>.
markt-asf commented on pull request #312:
URL: https://github.com/apache/tomcat/pull/312#issuecomment-653453487
Yes, DBCP2 changes need to be made upstream.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] rmaucher commented on pull request #312: Use StringBuilder instead of StringBuffer
Posted by GitBox <gi...@apache.org>.
rmaucher commented on pull request #312:
URL: https://github.com/apache/tomcat/pull/312#issuecomment-653402387
The "Append character instead of String when possible" seems wrong to me, and you're editing generated code there.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on pull request #312: Use StringBuilder instead of StringBuffer
Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #312:
URL: https://github.com/apache/tomcat/pull/312#issuecomment-653439627
Thanks, @pzygielo !
Also these seem to be the generated classes!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on pull request #312: Use StringBuilder instead of StringBuffer
Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #312:
URL: https://github.com/apache/tomcat/pull/312#issuecomment-653404420
@rmaucher Could you please comment on the changed lines ?
I am not sure which code is generated and which `append(char)` might be wrong.
Thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g merged pull request #312: Use StringBuilder instead of StringBuffer
Posted by GitBox <gi...@apache.org>.
martin-g merged pull request #312:
URL: https://github.com/apache/tomcat/pull/312
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on pull request #312: Use StringBuilder instead of StringBuffer
Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #312:
URL: https://github.com/apache/tomcat/pull/312#issuecomment-653508943
OK. I've reverted the DBCP classes!
And made some more changes from `.append(String)` to `.append(char)`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on pull request #312: Use StringBuilder instead of StringBuffer
Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #312:
URL: https://github.com/apache/tomcat/pull/312#issuecomment-653443649
I've reverted the changes to the generated classes.
What about the DBCP2 classes ? Maybe they should be changed upstream and then copied over here ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org