Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.7.0
    • Component/s: tserver
    • Labels:
      None

      Activity

      Hide
      Vikram Srivastava added a comment -

      Attached patch where class is moved out. Also added a unit test.

      Question: Do we need to retain Environment.getAuthorizations? It's not being used anywhere.

      Show
      Vikram Srivastava added a comment - Attached patch where class is moved out. Also added a unit test. Question: Do we need to retain Environment.getAuthorizations? It's not being used anywhere.
      Hide
      Eric Newton added a comment -

      You are going to want to do any work on ACCUMULO-2255 (or the sub-tickets) in a branch. I would actively discourage committers from applying refactorings in master before 1.6.0 is released. Merging changes through 1.4, 1.5 and 1.6 is hard enough as it is.

      Environment.getAuthorizations is in the public API, but since it has been deprecated for one release (the upcoming 1.6 release), it could be removed. There's been some (offline) discussion of being less aggressive removing deprecated APIs. I would leave it out for now.

      Show
      Eric Newton added a comment - You are going to want to do any work on ACCUMULO-2255 (or the sub-tickets) in a branch. I would actively discourage committers from applying refactorings in master before 1.6.0 is released. Merging changes through 1.4, 1.5 and 1.6 is hard enough as it is. Environment.getAuthorizations is in the public API, but since it has been deprecated for one release (the upcoming 1.6 release), it could be removed. There's been some (offline) discussion of being less aggressive removing deprecated APIs. I would leave it out for now.
      Hide
      Vikram Srivastava added a comment -

      Eric Newton I wanted to add more unit tests and make code better (e.g. not having public fields getting modified directly). Do you think it's better if I make all these changes in master itself, and then move all classes out once 1.6 is released?

      Show
      Vikram Srivastava added a comment - Eric Newton I wanted to add more unit tests and make code better (e.g. not having public fields getting modified directly). Do you think it's better if I make all these changes in master itself, and then move all classes out once 1.6 is released?
      Hide
      Eric Newton added a comment -

      I would keep it all in a remote branch/github. I know that worked well for some big changes (like namespaces) that were done by a non-committer. It allowed for distributed review, and easy ongoing merges.

      Show
      Eric Newton added a comment - I would keep it all in a remote branch/github. I know that worked well for some big changes (like namespaces) that were done by a non-committer. It allowed for distributed review, and easy ongoing merges.
      Hide
      Vikram Srivastava added a comment -

      I've created a remote branch at https://github.com/vickyuec/accumulo and will be pushing changes related to ACCUMULO-2255 here.

      Show
      Vikram Srivastava added a comment - I've created a remote branch at https://github.com/vickyuec/accumulo and will be pushing changes related to ACCUMULO-2255 here.
      Hide
      ASF subversion and git services added a comment -

      Commit dbb07f3739b2492c569b14c13e35f32e32927d72 in accumulo's branch refs/heads/master from Eric Newton
      [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=dbb07f3 ]

      ACCUMULO-2257 adding test from Vikram Srivastava's patch

      Show
      ASF subversion and git services added a comment - Commit dbb07f3739b2492c569b14c13e35f32e32927d72 in accumulo's branch refs/heads/master from Eric Newton [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=dbb07f3 ] ACCUMULO-2257 adding test from Vikram Srivastava's patch
      Hide
      ASF subversion and git services added a comment -

      Commit 42c1e64f395bc15ba67acc6d2bb1110dc47fb695 in accumulo's branch refs/heads/master from Eric Newton
      [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=42c1e64 ]

      ACCUMULO-2257 adding test from Vikram Srivastava's patch

      Show
      ASF subversion and git services added a comment - Commit 42c1e64f395bc15ba67acc6d2bb1110dc47fb695 in accumulo's branch refs/heads/master from Eric Newton [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=42c1e64 ] ACCUMULO-2257 adding test from Vikram Srivastava's patch
      Hide
      Eric Newton added a comment -

      Vikram Srivastava, if there's more that you'd like to do, please re-open.

      Show
      Eric Newton added a comment - Vikram Srivastava , if there's more that you'd like to do, please re-open.

        People

        • Assignee:
          Vikram Srivastava
          Reporter:
          Vikram Srivastava
        • Votes:
          0 Vote for this issue
          Watchers:
          3 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development