Weird, I don't get it as you can check at
https://issues.apache.org/jira/secure/attachment/12994793/OFBIZ-9352-not-used.png
Jacques
Le 27/02/2020 à 15:03, Michael Brohl a écrit :
Jacques,
I know and used this feature on demo-trunk. If you select the AccountingUiLabels.xml and search for unused labels, you get
AccountingDeleteRateAmount among others.
Using a find for *all* files with the unused labels checkbox did not display
any results for me.
Did not dig further on the logic to find those unused labels, maybe later.
Regards,
Michael Brohl
ecomify GmbH - www.ecomify.de
Am 27.02.20 um 14:53 schrieb Jacques Le Roux:
Michael,
Inline,
Le 27/02/2020 à 13:35, Michael Brohl a écrit :
You simply cannot rely on the LabelManager itself. It shows unused labels which
are used [1].
Example: AccountingErrorUiLabels.xml#AccountingDeleteRateAmount is shown as
unused on demo-trunk but is used in RateServices.groovy.
I did not have AccountingDeleteRateAmount in the list of 250 missing labels on
trunk demo.
I randomly picked a couple of other missing labels and did not find them but in
the AccountingUiLabels.xml file
This said to be sure I'd need to check 250 instances, nobody would do that. As you said, we need another tool. It would be used after automatically
collected unused labels enhancing "Not used labels" Label Manager feature.
Actually it's already what does "Not used labels" Label Manager feature ;) It's LabelManagerFactory:findMatchingLabels and depends on
onlyNotUsedLabels var. If you need more information please let me know.
How would you envision a tool to check its result, which may be indeed incomplete and even possibly wrong (though we made good progress last time,
see OFBIZ-8154)
So steps 1 and 2 are necessary because changes should not introduce regressions
(better unsed labels than missing labels).
IMO this check cannot be burdened upon the committer (if another contributor provided the patch).The responsibility to thoroughly check if a
removed label is in fact unused should be on the contributor. Which means he should prove which steps he took to check each removed label [2].
Yes, that's why, after being bitten once or twice, I'm now reluctant to remove
"unused" labels :/.
Jacques
Step 3 is not something we should make a general rule. I agree with Jacques
that this is unreasonable effort.
Thanks,
Michael Brohl
ecomify GmbH - www.ecomify.de
[1] I did not check how the LabelManager detects unused labels, can someone
briefly explain out of mind?
[2] We might need a mechanism or script to check a list of labels reported by
the UILabelManager against the codebase. Doing this by hand is tedious.
Am 27.02.20 um 12:45 schrieb Jacques Le Roux:
Hi,
With OFBIZ-9352 (under OFBIZ-10565) Pierre Smits propose to remove unused
labels from AccountingUiLabels.xml
This morning I looked at the related PR (17) and, using Label Manager (in Webtools) found that there are much more unused labels than those
Pierre proposes to remove.
I checked the 5 1st ones locally in my IDE and they are indeed unused (no
references at all but in the label file).
Now we had already a talk with Pierre, Scott and Michael about removing labels
in OFBIZ-9352.
Scott, Michael, and I in a less measure, are cautious about the reasons to remove
"unused" labels and you can read in OFBIZ-9352.
To define an unused labels I repeat what I said above:
An unused label is a label reported by the Label Manager as unused that can be
checked as unused by hand.
As you can see with my answer to Scoot in OFBIZ-9352, more work may be needed
if we want to trace why a label is unused.
My question, is should we remove unused labels?
And if we do so, which steps should me required:
1. Label Manager
2. Check by hand locally
3. Trace the reason
Obviously if we require the 3 steps nobody will do it (will you?).
Even the 2 steps are much work for a little benefit
So what? Should we close Jira issues related to removing unused labels as won't do.
Should they stay for the "eternity"?
About the Label Manager "unused labels" option, I think it can be still useful
in custom projects, but, as shown above, tricky OOTB.
Jacques
