[DISCUSSION] Should we clean labels?
Locked
[DISCUSSION] Should we clean labels?
 
Administrator
|
Re: [DISCUSSION] Should we clean labels?
|
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. 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]. 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 > > |
smime.p7s (5K)
Re: [DISCUSSION] Should we clean labels?
|
|
Re: [DISCUSSION] Should we clean labels?
|
|
Re: [DISCUSSION] Should we clean labels?
|
|
Re: [DISCUSSION] Should we clean labels?
|
Administrator
|
Re: [DISCUSSION] Should we clean labels?
|
|
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 >>> >>> >> |
Re: [DISCUSSION] Should we clean labels?
|
|
In reply to this post by Pierre Smits-3
Saying is not proving IMO.
The contributor should at least provide information *how* he did the checks along with the patch to help the committer decide. That is very reasonable to ask for. Low hanging fruits can also be foul. It does not help anyone to commit a patch to remove labels and get one or more Jira bug reports back, reporting missing labels. Michael ecomify GmbH - www.ecomify.de Am 27.02.20 um 14:42 schrieb Pierre Smits: > Re: 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]. > > We can all guess what the comment with the patch will be: 'I did the manual > check(s) and I found it only in <foo>Labels.xml'. > What then? > > 1. Asking for more details? Or screenshots? Or log excerpts? > 2. A statement of distrust towards the fellow contributor? > > > Don't require more than is reasonable for this kind of low hanging fruit. > |
Re: [DISCUSSION] Should we clean labels?
|
Administrator
|
| Free forum by Nabble | Edit this page |