Details

    • Improvement
    • Status: Closed
    • Blocker
    • Resolution: Implemented
    • Trunk
    • 17.12.01
    • None
    • None

    Description

      Following discussion at http://markmail.org/message/ut74mzptruubw4ty we have decided to

      1. have VERSION file at project root to store released version.
      2. Render the version in the applications footer template as a HTML comment.

      Also mbrohl suggested that

      1. it can be automatically set by (custom) builds to be generated.
      2. It should be made configurable in general.properties to prevent to show it if the user does not want this.

      Attachments

        1. OFBIZ-10141_Add_VERSION_file_simpler_solution.patch
          1 kB
          Michael Brohl
        2. OFBIZ-10141.patch
          5 kB
          Jacques Le Roux

        Activity

          Here is a patch I propose.

          The info is only rendered when using the specific versionInfoFooter Gradle task. The usage of this task is mostly for support requests when asking the user for version and the VERSION file is not accessible. So we don't need to use a general.properties which would complicate things a bit as suggested by Michael's point 2.

          When svnInfoFooter or gitInfoFooter is used the version info is removed.

          I have not implemented Michael's point 1 but we need to add something about updating the VERSION file in https://cwiki.apache.org/confluence/display/OFBIZ/Release+Management+Guide+for+OFBiz

          Please complete if I missed something, thanks

          jleroux Jacques Le Roux added a comment - Here is a patch I propose. The info is only rendered when using the specific versionInfoFooter Gradle task. The usage of this task is mostly for support requests when asking the user for version and the VERSION file is not accessible. So we don't need to use a general.properties which would complicate things a bit as suggested by Michael's point 2. When svnInfoFooter or gitInfoFooter is used the version info is removed. I have not implemented Michael's point 1 but we need to add something about updating the VERSION file in https://cwiki.apache.org/confluence/display/OFBIZ/Release+Management+Guide+for+OFBiz Please complete if I missed something, thanks
          mbrohl Michael Brohl added a comment -

          I'll have some remarks and questions but need some time for it over the weekend.

          mbrohl Michael Brohl added a comment - I'll have some remarks and questions but need some time for it over the weekend.

          No hurry, please take your time. I'll appreciate all reviews BTW

          jleroux Jacques Le Roux added a comment - No hurry, please take your time. I'll appreciate all reviews BTW

          Hi Michael,

          Any chances?

          jleroux Jacques Le Roux added a comment - Hi Michael, Any chances?

          I set the priority as blocker because we need to decide what we will do before next release.

          jleroux Jacques Le Roux added a comment - I set the priority as blocker because we need to decide what we will do before next release.
          mbrohl Michael Brohl added a comment -

          Hi Jacques,

          I think we can do this in a much simpler way. We can have the VERSION file and just add it's content in the footer section, maybe behind the copyright information.

          This would not require an include of a VERSION.ftl, new tasks etc.

          Let's keep it simple.

          Regards,

          Michael

          mbrohl Michael Brohl added a comment - Hi Jacques, I think we can do this in a much simpler way. We can have the VERSION file and just add it's content in the footer section, maybe behind the copyright information. This would not require an include of a VERSION.ftl, new tasks etc. Let's keep it simple. Regards, Michael

          Hi Michael,

          Since we decided to have a version file before next release, it needs to be actionnable. So please provide a patch which implements your idea, we can then discuss the pros and cons of both ways, thanks.

          jleroux Jacques Le Roux added a comment - Hi Michael, Since we decided to have a version file before next release, it needs to be actionnable. So please provide a patch which implements your idea, we can then discuss the pros and cons of both ways, thanks.
          mbrohl Michael Brohl added a comment -

          I've added a simpler solution as a patch. It just shows the usage in the rainbowstone theme, I'll complete it when it gets accepted by the community.

          The solution works with just a VERSION file and a small addition to every footer file in the templates. No build.gradle enhancements or additional ftl files needed.

          mbrohl Michael Brohl added a comment - I've added a simpler solution as a patch. It just shows the usage in the rainbowstone theme, I'll complete it when it gets accepted by the community. The solution works with just a VERSION file and a small addition to every footer file in the templates. No build.gradle enhancements or additional ftl files needed.
          mbrohl Michael Brohl added a comment -

          Changed patch file format and reattached the patch.

          mbrohl Michael Brohl added a comment - Changed patch file format and reattached the patch.
          mbrohl Michael Brohl added a comment -

          Additional remark: I don't think that we need any user configuration options to show/not show the version as opposed to my earlier ideas on this topic. Too complicated and not of much value.

          mbrohl Michael Brohl added a comment - Additional remark: I don't think that we need any user configuration options to show/not show the version as opposed to my earlier ideas on this topic. Too complicated and not of much value.
          mbrohl Michael Brohl added a comment -

          Any thoughts on the patches provided?

          mbrohl Michael Brohl added a comment - Any thoughts on the patches provided?
          nmalin Nicolas Malin added a comment -

          Any thoughts on the patches provided?{quote}

          I like , I think we can commit this on released version and trunk

          nmalin Nicolas Malin added a comment - Any thoughts on the patches provided?{quote} I like , I think we can commit this on released version and trunk
          mbrohl Michael Brohl added a comment -

          We have two different proposals/ patches so we need some opinions which one we want to commit....

          mbrohl Michael Brohl added a comment - We have two different proposals/ patches so we need some opinions which one we want to commit....

          I agree Michael's patch is simpler. The only drawback is that the version number is hidden "deeply" in CommonRelease label and needs a running version. Where a VERSION file is easier for any user to read and when needed given (mostly to us when helping on user ML). Also changing the CommonRelease label can be forgotten, but changing the VERSION file also.

          So all in all, I have no strong opinion anyway and I'd like more opinions than just Michael's and mine

          jleroux Jacques Le Roux added a comment - I agree Michael's patch is simpler. The only drawback is that the version number is hidden "deeply" in CommonRelease label and needs a running version. Where a VERSION file is easier for any user to read and when needed given (mostly to us when helping on user ML). Also changing the CommonRelease label can be forgotten, but changing the VERSION file also. So all in all, I have no strong opinion anyway and I'd like more opinions than just Michael's and mine
          mbrohl Michael Brohl added a comment -

          Please have a closer look at my patch, it is using a VERSION file...

          mbrohl Michael Brohl added a comment - Please have a closer look at my patch, it is using a VERSION file...

          Right, and CommonRelease is just containing the text "Release" so your solution seems indeed better. We will always have the release number showing in footer, but that seems like a good idea.
          I have though still a concern: only the current default theme is addressed

          jleroux Jacques Le Roux added a comment - Right, and CommonRelease is just containing the text "Release" so your solution seems indeed better. We will always have the release number showing in footer, but that seems like a good idea. I have though still a concern: only the current default theme is addressed
          mbrohl Michael Brohl added a comment -

          Yes, intentionally. See my comment on 20/Feb/18 17:14

          I'll complete this when we decide to use this approach, trying to be efficient

          mbrohl Michael Brohl added a comment - Yes, intentionally. See my comment on 20/Feb/18 17:14 I'll complete this when we decide to use this approach, trying to be efficient

          Hi Michael,

          Any chances?

          jleroux Jacques Le Roux added a comment - Hi Michael, Any chances?
          mbrohl Michael Brohl added a comment -

          I'll try do implement this in the next couple of days.

          mbrohl Michael Brohl added a comment - I'll try do implement this in the next couple of days.
          mbrohl Michael Brohl added a comment -

          This is implemented in

          trunk r1833173,

          release17.12 r1833174

           

          Thanks Jacques for the support.

          mbrohl Michael Brohl added a comment - This is implemented in trunk r1833173, release17.12 r1833174   Thanks Jacques for the support.

          People

            mbrohl Michael Brohl
            jleroux Jacques Le Roux
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: