Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: HBASE-14850
    • Fix Version/s: HBASE-14850
    • Component/s: None
    • Hadoop Flags:
      Reviewed

      Description

      There's a ton of files that just barf out all of folly, wangle, and hbase into the global namespace. We should use the using directive better than that when possible.

      1. HBASE-15602.HBASE-14850.patch
        74 kB
        Scott Hunt
      2. HBASE-15602.HBASE-14850.v2.patch
        74 kB
        Scott Hunt
      3. HBASE-15602.HBASE-14850.v3.patch
        90 kB
        Scott Hunt

        Activity

        Hide
        eclark Elliott Clark added a comment -

        This would be a good quick task if someone wanted to get started.

        Show
        eclark Elliott Clark added a comment - This would be a good quick task if someone wanted to get started.
        Hide
        Scott Scott Hunt added a comment -

        This is bigger than I would have liked.
        My first goal (though not in the description) was to remove usings from header files (because that can really do some bad/unexpected stuff.)
        Second goal was to remove or simplify most of the usings in source files, especially instances of using namespace.
        I tried to make sure all changes resulted in lines under 100 characters.

        I had to make a some guesses at what might be the desired style. Feel free to tell me where I need to make more changes. (i.e. I may have ended up under-using using statements for someone's taste.)

        Notes:
        I didn't touch any -test.cc files. I can do these if desired.
        While compiling, I encountered quite a few gcc warnings (primarily on constructor initializer order.) I have a further patch which attempts to clean all those up.

        Show
        Scott Scott Hunt added a comment - This is bigger than I would have liked. My first goal (though not in the description) was to remove usings from header files (because that can really do some bad/unexpected stuff.) Second goal was to remove or simplify most of the usings in source files, especially instances of using namespace. I tried to make sure all changes resulted in lines under 100 characters. I had to make a some guesses at what might be the desired style. Feel free to tell me where I need to make more changes. (i.e. I may have ended up under-using using statements for someone's taste.) Notes: I didn't touch any -test.cc files. I can do these if desired. While compiling, I encountered quite a few gcc warnings (primarily on constructor initializer order.) I have a further patch which attempts to clean all those up.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        0 docker 0m 13s Dockerfile '/home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/docker/Dockerfile' not found, falling back to built-in.
        -1 docker 5m 19s Docker failed to build yetus/hbase:date2017-05-30.



        Subsystem Report/Notes
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12869343/HBASE-15602.HBASE-14850.patch
        JIRA Issue HBASE-15602
        Console output https://builds.apache.org/job/PreCommit-HBASE-Build/6999/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. 0 docker 0m 13s Dockerfile '/home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/docker/Dockerfile' not found, falling back to built-in. -1 docker 5m 19s Docker failed to build yetus/hbase:date2017-05-30. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12869343/HBASE-15602.HBASE-14850.patch JIRA Issue HBASE-15602 Console output https://builds.apache.org/job/PreCommit-HBASE-Build/6999/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        enis Enis Soztutar added a comment -

        Scott Hunt thanks for working on this. It is a big patch, let me check whether it still applies. Probably needs rebasing.

        I had to make a some guesses at what might be the desired style. Feel free to tell me where I need to make more changes. (i.e. I may have ended up under-using using statements for someone's taste.)

        The standard practice is to use the clang-format tool to format the patches automatically so that everyone will use the same exact styling. There is a script under bin/format-code.sh which can be run inside the docker environment (bin/start-docker.sh).

        I didn't touch any -test.cc files. I can do these if desired.

        These are lower priority, but the cleaner the better. We can do a different issue.

        While compiling, I encountered quite a few gcc warnings (primarily on constructor initializer order.) I have a further patch which attempts to clean all those up.

        Sounds good. The patch is pretty big anyways, let's do a follow up patch.

        Show
        enis Enis Soztutar added a comment - Scott Hunt thanks for working on this. It is a big patch, let me check whether it still applies. Probably needs rebasing. I had to make a some guesses at what might be the desired style. Feel free to tell me where I need to make more changes. (i.e. I may have ended up under-using using statements for someone's taste.) The standard practice is to use the clang-format tool to format the patches automatically so that everyone will use the same exact styling. There is a script under bin/format-code.sh which can be run inside the docker environment (bin/start-docker.sh). I didn't touch any -test.cc files. I can do these if desired. These are lower priority, but the cleaner the better. We can do a different issue. While compiling, I encountered quite a few gcc warnings (primarily on constructor initializer order.) I have a further patch which attempts to clean all those up. Sounds good. The patch is pretty big anyways, let's do a follow up patch.
        Hide
        Scott Scott Hunt added a comment -

        Here's a rebased version (last commit = 517090a09a252b28cfda7083c91e53a4888bf9e2, HBASE-17860)
        A couple things changed upstream, but not much.

        Show
        Scott Scott Hunt added a comment - Here's a rebased version (last commit = 517090a09a252b28cfda7083c91e53a4888bf9e2, HBASE-17860 ) A couple things changed upstream, but not much.
        Hide
        Scott Scott Hunt added a comment -

        And of course, I jumped the gun before reading your entire comment. Sorry about that.
        Thanks for the tip about format-code.sh.

        patch v3 is after running format-code.sh

        Show
        Scott Scott Hunt added a comment - And of course, I jumped the gun before reading your entire comment. Sorry about that. Thanks for the tip about format-code.sh. patch v3 is after running format-code.sh
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        0 docker 0m 13s Dockerfile '/home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/docker/Dockerfile' not found, falling back to built-in.
        -1 docker 7m 48s Docker failed to build yetus/hbase:date2017-05-30.



        Subsystem Report/Notes
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12870460/HBASE-15602.HBASE-14850.v3.patch
        JIRA Issue HBASE-15602
        Console output https://builds.apache.org/job/PreCommit-HBASE-Build/7005/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. 0 docker 0m 13s Dockerfile '/home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/docker/Dockerfile' not found, falling back to built-in. -1 docker 7m 48s Docker failed to build yetus/hbase:date2017-05-30. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12870460/HBASE-15602.HBASE-14850.v3.patch JIRA Issue HBASE-15602 Console output https://builds.apache.org/job/PreCommit-HBASE-Build/7005/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        enis Enis Soztutar added a comment -

        Pushed this to the branch. Thanks for the patch Scott Hunt. I had to do two trivial fixes to the patch because some -test files were causing compilation failures.

        For next time, you can use

        buck test --all 
        

        to compile and execute all the unit tests (Makefile does not build the tests yet). The buck is best run on the docker environment.

        Show
        enis Enis Soztutar added a comment - Pushed this to the branch. Thanks for the patch Scott Hunt . I had to do two trivial fixes to the patch because some -test files were causing compilation failures. For next time, you can use buck test --all to compile and execute all the unit tests (Makefile does not build the tests yet). The buck is best run on the docker environment.

          People

          • Assignee:
            Scott Scott Hunt
            Reporter:
            eclark Elliott Clark
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development