groovy
  1. groovy
  2. GROOVY-3884

InvokerHelper.getVersion() should also work on Google App Engine

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.7-beta-2
    • Fix Version/s: 1.6.6, 1.7-rc-1
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      4

      Description

      Right now, InvokerHelper.getVersion() does nothing on GAE.
      Talking about version numbers, I suggest to create a GroovyVersion class that is focused on version numbers and is clearly part of the public API. People shouldn't have to use InvokerHelper to retrieve the version number.

      1. 3884_v17x_GroovyVersion_Patch.txt
        6 kB
        Roshan Dawrani
      2. 3884_v17x_v2.txt
        6 kB
        Roshan Dawrani
      3. 3884_v17x_v3.txt
        9 kB
        Roshan Dawrani
      4. 3884_v17x.txt
        5 kB
        Roshan Dawrani

        Activity

        Hide
        Roshan Dawrani added a comment -

        Most of the suggestions from you guys make sense - it's just that one is slightly better than the other.

        While GroovySystem.getVersion() seemed fine, GroovyVersion.getCurrentVersion() seems even better.

        In the current patch, GroovyVersion is in org.codehaus.groovy.*, if we want the version from GroovyVersion, it should
        come out on the public API.

        Most of the things are in place now - we just need to freeze its face on the public API. Otherwise, by default, it will
        remain as GroovySystem.getVersion() as that is what is in the codebase right now.

        Please let me know if GroovyVersion and its functionality in today's patch are needed.

        Show
        Roshan Dawrani added a comment - Most of the suggestions from you guys make sense - it's just that one is slightly better than the other. While GroovySystem.getVersion() seemed fine, GroovyVersion.getCurrentVersion() seems even better. In the current patch, GroovyVersion is in org.codehaus.groovy.*, if we want the version from GroovyVersion, it should come out on the public API. Most of the things are in place now - we just need to freeze its face on the public API. Otherwise, by default, it will remain as GroovySystem.getVersion() as that is what is in the codebase right now. Please let me know if GroovyVersion and its functionality in today's patch are needed.
        Hide
        blackdrag blackdrag added a comment -

        how about doing now: GroovySystem.getVersionString() and decide for doing its own class later? Then we can still add a getVersion() method, a new class, or whatever

        Show
        blackdrag blackdrag added a comment - how about doing now: GroovySystem.getVersionString() and decide for doing its own class later? Then we can still add a getVersion() method, a new class, or whatever
        Hide
        Peter Niederwieser added a comment -

        If we postpone GroovyVersion, I suggest to stick to InvokerHelper.getVersion() for now, and just change the implementation. Merely "moving" the method to GroovySystem doesn't add much value. Better introduce GroovyVersion at some point.

        Show
        Peter Niederwieser added a comment - If we postpone GroovyVersion, I suggest to stick to InvokerHelper.getVersion() for now, and just change the implementation. Merely "moving" the method to GroovySystem doesn't add much value. Better introduce GroovyVersion at some point.
        Hide
        Roshan Dawrani added a comment -

        Every individual piece of it is ready.

        We can have GroovyVersion.getCurrentVersion() on the public API (with InvokerHelper.getVersion() deprecated for 1.7.x). With 1.7-rc-1 also, we should not have people access internal classes to access the version.

        One way or another, access point should be a public API class, I think.

        Show
        Roshan Dawrani added a comment - Every individual piece of it is ready. We can have GroovyVersion.getCurrentVersion() on the public API (with InvokerHelper.getVersion() deprecated for 1.7.x). With 1.7-rc-1 also, we should not have people access internal classes to access the version. One way or another, access point should be a public API class, I think.
        Hide
        Roshan Dawrani added a comment -

        I will not make any further changes on it unless a clear message comes out on this - which means that "String GroovySystem.getVersion()" will remain in API.

        Show
        Roshan Dawrani added a comment - I will not make any further changes on it unless a clear message comes out on this - which means that "String GroovySystem.getVersion()" will remain in API.

          People

          • Assignee:
            Roshan Dawrani
            Reporter:
            Peter Niederwieser
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: