You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "myskov (via GitHub)" <gi...@apache.org> on 2025/03/15 13:56:48 UTC
[PR] HDDS-12530. Enable LooseCoupling check in PMD [ozone]
myskov opened a new pull request, #8092:
URL: https://github.com/apache/ozone/pull/8092
## What changes were proposed in this pull request?
I added LooseCoupling check in PMD ruleset. Vilotaions that require massive refactoring were suppressed.
It's a large commit, however, the changes are trivial. Let me know if you'd prefer me to break the PR into smaller parts.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-12530
## How was this patch tested?
Existing tests
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12530. Enable LooseCoupling check in PMD [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #8092:
URL: https://github.com/apache/ozone/pull/8092#issuecomment-2739073623
> Let me know if you'd prefer me to break the PR into smaller parts.
Let's limit the first change to test code, which triggers fewer CI checks. Follow-up can update all remaining items (non-test, plus any tests broken in the meantime, before adding the rule).
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12530. Enable LooseCoupling check in PMD [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #8092:
URL: https://github.com/apache/ozone/pull/8092#issuecomment-2786016810
/pending limit to tests
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12530. Enable LooseCoupling check in PMD [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #8092:
URL: https://github.com/apache/ozone/pull/8092#issuecomment-2734008006
> ```diff
> - private final ConcurrentHashMap<UUID, Long> commitInfoMap;
> + private final Map<UUID, Long> commitInfoMap;
> ```
>
> The class requires that the map is thread-safe, and the declared type helps enforce this. It is not true that any map implementation would suffice as the declaration implies.
It still should be declared as `ConcurrentMap`.
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12530. Enable LooseCoupling check in PMD [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on PR #8092:
URL: https://github.com/apache/ozone/pull/8092#issuecomment-2734037885
> It still should be declared as ConcurrentMap
Agreed, but does that still pass this check? In 243 in `XceiverClientRatis` it looks like it won't, although that is an area were I think the base `Map` interface is applicable. This is just one specific case too. Usage of `EnumMap` in declarations could go either way IMO. We would need to exhaustively go through the data structure hierarchies and define for each one how specific the class fields should be vs. the types used in the API. I'm not sure this is feasible unless PMD has some killer automation built in to help us.
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12530. Enable LooseCoupling check in PMD [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #8092:
URL: https://github.com/apache/ozone/pull/8092#issuecomment-2734339995
> Agreed, but does that still pass this check?
Yes, tried it.
It only flags concrete classes, any interface type will be OK:
https://github.com/pmd/pmd/blob/9f54da98f02ab2ba10bcf4a2e3c907647a0b486e/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/LooseCouplingRule.java#L80-L83
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12530. Enable LooseCoupling check in PMD [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8092:
URL: https://github.com/apache/ozone/pull/8092#issuecomment-2840517657
Thank you very much for the patch. I am closing this PR __temporarily__ as there was no activity recently and it is waiting for response from its author.
It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time.
It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs.
If you need ANY help to finish this PR, please [contact the community](https://github.com/apache/hadoop-ozone#contact) on the mailing list or the slack channel."
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-12530. Enable LooseCoupling check in PMD [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #8092: HDDS-12530. Enable LooseCoupling check in PMD
URL: https://github.com/apache/ozone/pull/8092
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org