New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JBTM 3089 (LRA code improvements) and JBTM-3413 (close resources) #2191
Conversation
Started testing this pull request with LRA profile: https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=openJDK11,label=jnlp-agent/297/ |
LRA profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=openJDK11,label=jnlp-agent/297/ |
Started testing this pull request with LRA profile: https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=openJDK11,label=jnlp-agent/299/ |
LRA profile tests passed - Job complete https://ci-jenkins-csb-narayana.apps.ocp-c1.prod.psi.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=openJDK11,label=jnlp-agent/299/ |
Thanks @mmusgrov for this PR! Could I ask you if you used a tool for improving format and comments? |
There is no reformatting involved (apart from the extra blank line after the SPDX id and in one or two other places, but minimal). That said I used Intelij warnings as my prompt in many places but those cover a lot more than just formatting. If you analyse each change on its own merit you should notice that they are improving the code and not reformatting it and that there is sufficient diversity in the change set that only an AI assistant would be able to duplicate that change set. We can pick on a random subset of the changes if you like and I will explain my justification for each of them, which ones in particular did you have in mind? |
Thanks Mike for your clarification. I said formatting because I can see some commented code blocks have been deleted (which is good) and some unused method have been deleted (which is super good). I asked only to have a general idea of your work, I need no justification ;) Thanks again for you work! +1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved! +1
@marcosgopen I updated the PR description since the change set also addresses JBTM-3413 |
https://issues.redhat.com/browse/JBTM-3089 (LRA code improvements)
https://issues.redhat.com/browse/JBTM-3413 (Close JAX-RS message entity input streams)
LRA !CORE !TOMCAT !AS_TESTS !RTS !JACOCO !XTS !QA_JTA !QA_JTS_OPENJDKORB !PERFORMANCE !DB_TESTS !mysql !db2 !postgres !oracle
This is a non-functional change to improve the readability and maintainability.