JBTM-3562 Child class fields should not shadow parent class fields #1932
Conversation
|
Author is not the 'narayana' contributor, to permit PR being run members of jbosstm can write comment of text: TESTIT |
1 similar comment
|
Author is not the 'narayana' contributor, to permit PR being run members of jbosstm can write comment of text: TESTIT |
|
@jmfinelli @mmusgrov Could you check and start the CI, please? Do the chosen runs cover enough? |
|
@boris-unckel I would select only CORE to test this modification. The list of profile that I would run is: Note: BTW, !NO_WIN means that the test profile WIN will be run (double negation). |
|
@jmfinelli Thanks, I have updated the comment above. |
|
Thanks @boris-unckel |
|
@mmusgrov I have found https://github.com/jbosstm/narayana/blob/5.12.4.Final/ArjunaCore/arjuna/tests/classes/com/hp/mwtests/ts/arjuna/abstractrecords/DisposeRecordUnitTest.java Is there more you would expect? If yes, do you have a sample? |
| @@ -1387,7 +1387,7 @@ synchronized final void destroyed () | |||
|
|
|||
| protected Hashtable modifyingActions = null; | |||
| protected Hashtable usingActions = null; | |||
| protected final Uid objectUid; | |||
There was a problem hiding this comment.
final cannot be removed. In fact, objectUid should never be changed once created.
| @@ -267,8 +267,6 @@ public DisposeRecord () | |||
| typeName = null; | |||
| targetParticipantStore = null; | |||
| } | |||
|
|
|||
| private Uid objectUid; | |||
There was a problem hiding this comment.
As a consequence of my previous comment (and for design reasons), this cannot be modified. For clarity sake, DisposeRecord.objectUid could be renamed to something that will not trigger warnings in code analysis tools...but:
- The new name should reflect the reasons why this variable was declared here (and those reasons can be found in the original design of Arjuna)
- I am not sure that other contributors/maintainers would agree with me
There was a problem hiding this comment.
I'd be fine renaming it. Something like "originalInstanceID" as the DisposeRecord is created to potentially manage the state of other StateManager objects, like TXOJ, in the event of crash recovery.
There was a problem hiding this comment.
@jmfinelli @nmcl Thanks for your feedback. I'm going to prepare a update for PR with renaming.
There was a problem hiding this comment.
Thanks @boris-unckel for updating your PR
Fixes https://issues.redhat.com/browse/JBTM-3562 Removes fields from child classes, where parent class provides the field. Rename fields where child field is correct.
boris-unckel
force-pushed
the
JBTM-3562_remove_shadow
branch
from
ee89e7c
to
c2b70d9
Compare
|
Author is not the 'narayana' contributor, to permit PR being run members of jbosstm can write comment of text: TESTIT |
1 similar comment
|
Author is not the 'narayana' contributor, to permit PR being run members of jbosstm can write comment of text: TESTIT |
|
TESTIT |
|
Started testing this pull request with MAIN profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk8.latest,label=linux/525/ |
|
Started testing this pull request with QA_JTA profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTA,jdk=jdk8.latest,label=linux/526/ |
|
Started testing this pull request with MAIN profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk8.latest,label=linux/526/ |
|
CORE profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk8.latest,label=linux/525/ |
|
CORE profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk8.latest,label=linux/526/ |
|
QA_JTA profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=QA_JTA,jdk=jdk8.latest,label=linux/526/ |
|
@boris-unckel I am not able to assign you the jira, will see if you are able to self assign it. |
|
@mmusgrov Thanks for merging. I don't have the necessary rights granted in general. This problem is a general one for RedHats Jira. I don't mind if you take it yourself or keep it empty (both happened in different projects). |
|
That's annoying. I will leave it blank for now but how did you manage to
get yourself assigned to WFLY jiras?
…On Wed, Dec 15, 2021 at 3:27 PM Boris Unckel ***@***.***> wrote:
@mmusgrov <https://github.com/mmusgrov> Thanks for merging. I don't have
the necessary rights granted in general. This problem is a general one for
RedHats Jira. I don't mind if you take it yourself or keep it empty (both
happened in different projects).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1932 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADYPNEINHMVV6B667ONHGLURCXWXANCNFSM5JHI2TXQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
Michael Musgrove
***@***.***
JBoss, by Red Hat
Registered Address: Red Hat Ltd, 6700 Cork Airport Business Park, Kinsale
Road, Co. Cork.
Registered in the Companies Registration Office, Parnell House, 14 Parnell
Square, Dublin 1, Ireland, No.304873
Directors:Michael Cunningham (USA), Vicky Wiseman (USA), Michael O'Neill,
Keith Phelan, Matt Parson (USA)
|
|
With a workaround: In the past a WFLY Jira had a button "Start progress" (or similar). This workflow included a assign. But direct assigning by maintainers did not work, too. |
|
JBTM issues also have a start progress button.
I have just re-opened the issue so will you try the button to see if it
populates the Assignee field. I will then resolve the issue again once
that's done.
…On Wed, Dec 15, 2021 at 8:09 PM Boris Unckel ***@***.***> wrote:
With a workaround: In the past a WFLY Jira had a button "Start progress"
(or similar). This workflow included a assign. But direct assigning by
maintainers did not work, too.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1932 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADYPNFT55HAMCBSCEO62CTURDYXVANCNFSM5JHI2TXQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
Michael Musgrove
***@***.***
JBoss, by Red Hat
Registered Address: Red Hat Ltd, 6700 Cork Airport Business Park, Kinsale
Road, Co. Cork.
Registered in the Companies Registration Office, Parnell House, 14 Parnell
Square, Dublin 1, Ireland, No.304873
Directors:Michael Cunningham (USA), Vicky Wiseman (USA), Michael O'Neill,
Keith Phelan, Matt Parson (USA)
|
|
@mmusgrov I have tried. But (for me?) there is only a "Log work" button, not "Start Progress". Please check this screenshot: https://ibb.co/fQ2Vxs9 |
|
Now it's getting really strange: The field Assignee shows "Unassigned". But filtering all issues with "Assigned to me" leads me to this one. |
|
@boris-unckel yes I have raised the issue elsewhere and will provide an update when it is resolved |
Fixes https://issues.redhat.com/browse/JBTM-3562
Removes fields from child classes, where parent class provides the field.
CORE !TOMCAT !AS_TESTS !RTS !JACOCO !XTS QA_JTA !QA_JTS_JACORB !QA_JTS_JDKORB !QA_JTS_OPENJDKORB !PERF !LRA NO_WIN !DB_TESTS !mysql !db2 !postgres !oracle