You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacques Le Roux <ja...@les7arts.com> on 2020/02/27 11:45:18 UTC
[DISCUSSION] Should we clean labels?
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?
Posted by Michael Brohl <mi...@ecomify.de>.
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?
Posted by Pierre Smits <pi...@apache.org>.
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.
Met vriendelijke groet,
Pierre Smits
*Proud* *contributor** of* Apache OFBiz <https://ofbiz.apache.org/> since
2008 (without privileges)
*Apache Trafodion <https://trafodion.apache.org>, Vice President*
*Apache Directory <https://directory.apache.org>, PMC Member*
Apache Incubator <https://incubator.apache.org>, committer
Apache Steve <https://steve.apache.org>, committer
On Thu, Feb 27, 2020 at 2:23 PM Pierre Smits <pi...@apache.org> wrote:
> Whatever the participation and the outcome of this discussion will be...
>
> In the past complaints (relating to the subject of labels) have been
> raised about:
>
> - patches too big (because of too much work to investigate, merges
> leading to conflicts in local repos), and
> - patches too small (asking for more work done before willingness to
> collaborate)
>
> Re: tracing the reason why something happened in the past
> This should not be a requirement (at least for aspects like unused labels
> typos and whitespace), as it places an burden on the contributor leading to
> a cost (effort) disproportionate to the benefit. Above that, it will feed
> the blame-game.
>
> Such complaints (and unreasonable requirements) are detrimental to the
> health of the project, and should not happen.
>
> Met vriendelijke groet,
>
> Pierre Smits
> *Proud* *contributor** of* Apache OFBiz <https://ofbiz.apache.org/> since
> 2008 (without privileges)
>
> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> *Apache Directory <https://directory.apache.org>, PMC Member*
> Apache Incubator <https://incubator.apache.org>, committer
> Apache Steve <https://steve.apache.org>, committer
>
>
> On Thu, Feb 27, 2020 at 1:58 PM Pierre Smits <pi...@apache.org>
> wrote:
>
>> Given that the OFBiz community consists of more than just a handful of
>> active PMC Members, Committers and other contributors, the question should
>> *not* just be: Should we clean labels'.
>>
>> But rather: Should we welcome contributions that only address the
>> cleaning of unused labels, and if so get them into the code base?
>>
>> The same question can be asked for contributions regarding:
>>
>> - typos
>> - unused code
>> - whitespace
>> - etc.
>>
>> My answer, for the benefit of the project and the appreciation felt by
>> the (potential) contributor and adopter, will always be: an unequivocal YES.
>> Not only for reasons stated above, but also for all other obvious
>> positive arguments. This should not be about catering to people NOT willing
>> to do stuff, even if it is not important enough for them or regarded as
>> menial by them. This is about catering to people willing to do even this
>> kind of stuff.
>>
>> And if there is no-one (among the pool of privileged contributors)
>> willing to do this kind of work, then the PMC should grant privileges to
>> those contributors willing to do that,
>>
>> Met vriendelijke groet,
>>
>> Pierre Smits
>> *Proud* *contributor** of* Apache OFBiz <https://ofbiz.apache.org/> since
>> 2008 (without privileges)
>>
>> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
>> *Apache Directory <https://directory.apache.org>, PMC Member*
>> Apache Incubator <https://incubator.apache.org>, committer
>> Apache Steve <https://steve.apache.org>, committer
>>
>>
>> On Thu, Feb 27, 2020 at 12:46 PM Jacques Le Roux <
>> jacques.le.roux@les7arts.com> wrote:
>>
>>> 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?
Posted by Pierre Smits <pi...@apache.org>.
Whatever the participation and the outcome of this discussion will be...
In the past complaints (relating to the subject of labels) have been raised
about:
- patches too big (because of too much work to investigate, merges
leading to conflicts in local repos), and
- patches too small (asking for more work done before willingness to
collaborate)
Re: tracing the reason why something happened in the past
This should not be a requirement (at least for aspects like unused labels
typos and whitespace), as it places an burden on the contributor leading to
a cost (effort) disproportionate to the benefit. Above that, it will feed
the blame-game.
Such complaints (and unreasonable requirements) are detrimental to the
health of the project, and should not happen.
Met vriendelijke groet,
Pierre Smits
*Proud* *contributor** of* Apache OFBiz <https://ofbiz.apache.org/> since
2008 (without privileges)
*Apache Trafodion <https://trafodion.apache.org>, Vice President*
*Apache Directory <https://directory.apache.org>, PMC Member*
Apache Incubator <https://incubator.apache.org>, committer
Apache Steve <https://steve.apache.org>, committer
On Thu, Feb 27, 2020 at 1:58 PM Pierre Smits <pi...@apache.org> wrote:
> Given that the OFBiz community consists of more than just a handful of
> active PMC Members, Committers and other contributors, the question should
> *not* just be: Should we clean labels'.
>
> But rather: Should we welcome contributions that only address the cleaning
> of unused labels, and if so get them into the code base?
>
> The same question can be asked for contributions regarding:
>
> - typos
> - unused code
> - whitespace
> - etc.
>
> My answer, for the benefit of the project and the appreciation felt by the
> (potential) contributor and adopter, will always be: an unequivocal YES.
> Not only for reasons stated above, but also for all other obvious positive
> arguments. This should not be about catering to people NOT willing to do
> stuff, even if it is not important enough for them or regarded as menial by
> them. This is about catering to people willing to do even this kind of
> stuff.
>
> And if there is no-one (among the pool of privileged contributors) willing
> to do this kind of work, then the PMC should grant privileges to those
> contributors willing to do that,
>
> Met vriendelijke groet,
>
> Pierre Smits
> *Proud* *contributor** of* Apache OFBiz <https://ofbiz.apache.org/> since
> 2008 (without privileges)
>
> *Apache Trafodion <https://trafodion.apache.org>, Vice President*
> *Apache Directory <https://directory.apache.org>, PMC Member*
> Apache Incubator <https://incubator.apache.org>, committer
> Apache Steve <https://steve.apache.org>, committer
>
>
> On Thu, Feb 27, 2020 at 12:46 PM Jacques Le Roux <
> jacques.le.roux@les7arts.com> wrote:
>
>> 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?
Posted by Pierre Smits <pi...@apache.org>.
Given that the OFBiz community consists of more than just a handful of
active PMC Members, Committers and other contributors, the question should
*not* just be: Should we clean labels'.
But rather: Should we welcome contributions that only address the cleaning
of unused labels, and if so get them into the code base?
The same question can be asked for contributions regarding:
- typos
- unused code
- whitespace
- etc.
My answer, for the benefit of the project and the appreciation felt by the
(potential) contributor and adopter, will always be: an unequivocal YES.
Not only for reasons stated above, but also for all other obvious positive
arguments. This should not be about catering to people NOT willing to do
stuff, even if it is not important enough for them or regarded as menial by
them. This is about catering to people willing to do even this kind of
stuff.
And if there is no-one (among the pool of privileged contributors) willing
to do this kind of work, then the PMC should grant privileges to those
contributors willing to do that,
Met vriendelijke groet,
Pierre Smits
*Proud* *contributor** of* Apache OFBiz <https://ofbiz.apache.org/> since
2008 (without privileges)
*Apache Trafodion <https://trafodion.apache.org>, Vice President*
*Apache Directory <https://directory.apache.org>, PMC Member*
Apache Incubator <https://incubator.apache.org>, committer
Apache Steve <https://steve.apache.org>, committer
On Thu, Feb 27, 2020 at 12:46 PM Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:
> 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?
Posted by Jacques Le Roux <ja...@les7arts.com>.
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
>>>>
>>>>
>>>
>
Re: [DISCUSSION] Should we clean labels?
Posted by Michael Brohl <mi...@ecomify.de>.
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?
Posted by Jacques Le Roux <ja...@les7arts.com>.
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?
Posted by Michael Brohl <mi...@ecomify.de>.
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
>
>