Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-5994

Have view-entity AgreementAndRole removed

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Done
    • Affects Version/s: Trunk, 16.11.01
    • Fix Version/s: 16.11.01
    • Component/s: party
    • Sprint:
      Bug Crush Event - 21/2/2015

      Description

      The view-entity is not referenced in any Screen, Form and Service definition. Having this in the entity-model is superfluous.

        Activity

        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        This was introduced by OFBIZ-1794, not sure we want to remove it, Jacopo Cappellato, Harsh Vijaywargiya?

        Show
        jacques.le.roux Jacques Le Roux added a comment - This was introduced by OFBIZ-1794 , not sure we want to remove it, Jacopo Cappellato , Harsh Vijaywargiya ?
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Moreover other people might use it. I prefer to close as won't fix!

        Show
        jacques.le.roux Jacques Le Roux added a comment - Moreover other people might use it. I prefer to close as won't fix!
        Hide
        pfm.smits Pierre Smits added a comment - - edited

        Other people?

        So, you're suggesting to not clean up unused code?

        Show
        pfm.smits Pierre Smits added a comment - - edited Other people? So, you're suggesting to not clean up unused code?
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        I mean that people could have used it in custom projects and removing it from the trunk might impair their projects

        "The road to hell is paved with good intentions"

        Show
        jacques.le.roux Jacques Le Roux added a comment - I mean that people could have used it in custom projects and removing it from the trunk might impair their projects "The road to hell is paved with good intentions"
        Hide
        pfm.smits Pierre Smits added a comment -

        Then I suggest to let them come forward and explain the dependency before you close the issue as a won't fix. Now you know nothing and make assumptions.

        Show
        pfm.smits Pierre Smits added a comment - Then I suggest to let them come forward and explain the dependency before you close the issue as a won't fix. Now you know nothing and make assumptions.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Are you serious?

        Show
        jacques.le.roux Jacques Le Roux added a comment - Are you serious?
        Hide
        gil portenseigne Gil Portenseigne added a comment -

        This argument is good for branches, but i find it strange for trunk. The use of trunk code from potential users, shouldn't block cleaning or improving the project.

        It not the first time we talk about project based on trunk, that should be done at their own risk.

        Show
        gil portenseigne Gil Portenseigne added a comment - This argument is good for branches, but i find it strange for trunk. The use of trunk code from potential users, shouldn't block cleaning or improving the project. It not the first time we talk about project based on trunk, that should be done at their own risk.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        That's true, but it exists and why annoy them? Of course we can't know before since few people seriously follows things...
        I don't see a problem keeping that

        Show
        jacques.le.roux Jacques Le Roux added a comment - That's true, but it exists and why annoy them? Of course we can't know before since few people seriously follows things... I don't see a problem keeping that
        Hide
        gil portenseigne Gil Portenseigne added a comment -

        No problem either to me, it's a very basic/usable view. I just pointed the argument

        Show
        gil portenseigne Gil Portenseigne added a comment - No problem either to me, it's a very basic/usable view. I just pointed the argument
        Hide
        jacopoc Jacopo Cappellato added a comment -

        +1 for removing it, if it is not used by OFBiz.

        Show
        jacopoc Jacopo Cappellato added a comment - +1 for removing it, if it is not used by OFBiz.
        Hide
        pfm.smits Pierre Smits added a comment -

        Reopening the issue, to allow any interested party to create functionality utilising the entity.

        If not done within a reasonable amount of time, this issue should be considered a reminder to remove it before the creation of the next release branch.

        Show
        pfm.smits Pierre Smits added a comment - Reopening the issue, to allow any interested party to create functionality utilising the entity. If not done within a reasonable amount of time, this issue should be considered a reminder to remove it before the creation of the next release branch.
        Hide
        gavin.mabie@urbannex.co.za Gavin Mabie added a comment -

        (resend as requested)
        I'm sure that there are a few of these cases - i.e. where entities are not referenced in any Screen, Form and Service definition. I suspect that this would be the case with especially view-entities which are used mainly in summary/report type screens and less so in transactional screens. I just did a quick search on the BenefitTypeAndParty view-entity and could not find any reference to it in Screen, Form or Service definitions. Does this qualify it for removal/deprecation? I don't think so. Just because it has not being used does not necessarily translate into it not being useful. We should approach deprecating entities with caution, particularly those which related to core ERP functionalities. Long story short - I don't think its a good idea to deprecate this entity or any other at this stage. Entity deprecation should be done at a customization level and not at a Project level.

        I don't know if we have one - but a Deprecation Policy might be helpful when dealing with these kinds of issues. As an example see - https://wiki.eclipse.org/Eclipse/API_Central/Deprecation_Policy

        Gavin

        Show
        gavin.mabie@urbannex.co.za Gavin Mabie added a comment - (resend as requested) I'm sure that there are a few of these cases - i.e. where entities are not referenced in any Screen, Form and Service definition. I suspect that this would be the case with especially view-entities which are used mainly in summary/report type screens and less so in transactional screens. I just did a quick search on the BenefitTypeAndParty view-entity and could not find any reference to it in Screen, Form or Service definitions. Does this qualify it for removal/deprecation? I don't think so. Just because it has not being used does not necessarily translate into it not being useful. We should approach deprecating entities with caution, particularly those which related to core ERP functionalities. Long story short - I don't think its a good idea to deprecate this entity or any other at this stage. Entity deprecation should be done at a customization level and not at a Project level. I don't know if we have one - but a Deprecation Policy might be helpful when dealing with these kinds of issues. As an example see - https://wiki.eclipse.org/Eclipse/API_Central/Deprecation_Policy Gavin
        Hide
        jacopoc Jacopo Cappellato added a comment -

        Deprecation policies for a view entity don't make much sense because removing it will not impact the data but only the data extraction code.

        Show
        jacopoc Jacopo Cappellato added a comment - Deprecation policies for a view entity don't make much sense because removing it will not impact the data but only the data extraction code.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        I agree that https://cwiki.apache.org/confluence/display/OFBIZ/Revisions+Requiring+Data+Migration+-+upgrade+ofbiz is not directly concerned. A word about it would not harm though, I can't think of a better place

        Show
        jacques.le.roux Jacques Le Roux added a comment - I agree that https://cwiki.apache.org/confluence/display/OFBIZ/Revisions+Requiring+Data+Migration+-+upgrade+ofbiz is not directly concerned. A word about it would not harm though, I can't think of a better place
        Hide
        pfm.smits Pierre Smits added a comment -

        This patch addresses the issue.

        Show
        pfm.smits Pierre Smits added a comment - This patch addresses the issue.
        Hide
        pfm.smits Pierre Smits added a comment -

        Ping?

        Show
        pfm.smits Pierre Smits added a comment - Ping?
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Pierre,

        your modified patch is in trunk at revision 1756365.

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Pierre, your modified patch is in trunk at revision 1756365.

          People

          • Assignee:
            pfm.smits Pierre Smits
            Reporter:
            pfm.smits Pierre Smits
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development

                Agile