HBase
  1. HBase
  2. HBASE-4611

Add support for Phabricator/Differential as an alternative code review tool

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.92.0, 0.94.0
    • Component/s: None
    • Labels:
      None

      Description

      From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook."

      It's open source so pretty much anyone could host an instance of this software.

      To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org).

      We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase.

      1. ASF.LICENSE.NOT.GRANTED--HBASE-4611.D423.2.patch
        0.5 kB
        Phabricator
      2. ASF.LICENSE.NOT.GRANTED--HBASE-4611.D423.1.patch
        0.5 kB
        Phabricator
      3. ASF.LICENSE.NOT.GRANTED--D207.1.patch
        1 kB
        Phabricator
      4. ASF.LICENSE.NOT.GRANTED--D201.1.patch
        0.3 kB
        Phabricator
      5. ASF.LICENSE.NOT.GRANTED--D165.2.patch
        0.4 kB
        Phabricator
      6. ASF.LICENSE.NOT.GRANTED--D189.1.patch
        0.3 kB
        Phabricator
      7. ASF.LICENSE.NOT.GRANTED--D177.2.patch
        0.6 kB
        Phabricator
      8. ASF.LICENSE.NOT.GRANTED--D183.1.patch
        0.6 kB
        Phabricator
      9. ASF.LICENSE.NOT.GRANTED--D171.1.patch
        0.2 kB
        Phabricator
      10. ASF.LICENSE.NOT.GRANTED--D177.1.patch
        0.6 kB
        Phabricator
      11. ASF.LICENSE.NOT.GRANTED--D165.1.patch
        0.3 kB
        Phabricator
      12. ASF.LICENSE.NOT.GRANTED--D153.1.patch
        2 kB
        Phabricator
      13. ASF.LICENSE.NOT.GRANTED--D21.1.patch
        0.5 kB
        noreply@reviews.facebook.net
      14. ASF.LICENSE.NOT.GRANTED--D21.1.patch
        0.5 kB
        noreply@reviews.facebook.net

        Issue Links

          Activity

          Hide
          Jonathan Gray added a comment -

          In addition to being a (better) code review tool, the Phabricator suite also includes stuff like repo/revision browsing, nice command-line tools, pastebin, etc. which should be available for the HBase repos.

          Show
          Jonathan Gray added a comment - In addition to being a (better) code review tool, the Phabricator suite also includes stuff like repo/revision browsing, nice command-line tools, pastebin, etc. which should be available for the HBase repos.
          Hide
          Jonathan Gray added a comment -

          Should we change the display name of the reviews.facebook.net account from John Sichi?

          Show
          Jonathan Gray added a comment - Should we change the display name of the reviews.facebook.net account from John Sichi?
          Hide
          Marek Sapota added a comment -

          We probably should=) Do you know who might be able to do that? Since the account was auto-created with a phony email we would need an administrator help to access it.

          Show
          Marek Sapota added a comment - We probably should=) Do you know who might be able to do that? Since the account was auto-created with a phony email we would need an administrator help to access it.
          Hide
          Liyin Tang added a comment -

          Very Awesome. I have tried to created one review on Phabricator

          Show
          Liyin Tang added a comment - Very Awesome. I have tried to created one review on Phabricator
          Hide
          Jonathan Gray added a comment -

          @Marek, we could file an INFRA task. Or we could create a new account? Also, there seems to be something with URL translation (JIRA is treating the <a> tag as escaped so actually showing it, and then converting straight text URLs to hyperlinks).

          Show
          Jonathan Gray added a comment - @Marek, we could file an INFRA task. Or we could create a new account? Also, there seems to be something with URL translation (JIRA is treating the <a> tag as escaped so actually showing it, and then converting straight text URLs to hyperlinks).
          Hide
          Jonathan Gray added a comment -

          Looks like you're one step ahead on the <a> tags, thanks!

          Show
          Jonathan Gray added a comment - Looks like you're one step ahead on the <a> tags, thanks!
          Hide
          Marek Sapota added a comment -

          I believe accounts need a valid email address so either way we would need some help - I'll create an INFRA task.

          JIRA saves descriptions as HTML, hopefully some simple replacements will be enough to convert them to plain text.

          Show
          Marek Sapota added a comment - I believe accounts need a valid email address so either way we would need some help - I'll create an INFRA task. JIRA saves descriptions as HTML, hopefully some simple replacements will be enough to convert them to plain text.
          Hide
          John Sichi added a comment -

          Hey HBase peeps (this is the real me writing, I swear):

          Besides committing the project-specific .arcconfig file, we're also going to need to make available the arcanist extensions Marek has developed. Most of this will be the same across Hive/HBase/HCatalog/etc (although each project might have its own additional extensions for stuff like linting rules).

          We could just commit these redundantly to each project, but that would make maintaining them a pain.

          Alternatively, we can maintain the common extensions somewhere independently (e.g. github or apache extras), and have each project pull it in via a build command. For example, in Hive I might run "ant arc-install" in a new sandbox to get it; HBase would supply a mvn command.

          Thoughts?

          Show
          John Sichi added a comment - Hey HBase peeps (this is the real me writing, I swear): Besides committing the project-specific .arcconfig file, we're also going to need to make available the arcanist extensions Marek has developed. Most of this will be the same across Hive/HBase/HCatalog/etc (although each project might have its own additional extensions for stuff like linting rules). We could just commit these redundantly to each project, but that would make maintaining them a pain. Alternatively, we can maintain the common extensions somewhere independently (e.g. github or apache extras), and have each project pull it in via a build command. For example, in Hive I might run "ant arc-install" in a new sandbox to get it; HBase would supply a mvn command. Thoughts?
          Hide
          Liyin Tang added a comment -

          One problem I just found is the patch automatically attached by phabricator does not have the ASF license. Also this patch cannot be deleted.

          Show
          Liyin Tang added a comment - One problem I just found is the patch automatically attached by phabricator does not have the ASF license. Also this patch cannot be deleted.
          Hide
          John Sichi added a comment -

          @Liyin: yeah, for now we haven't thought of a good solution for the ASF grant yet. I'm not sure but I'm guessing it would need to be done explicitly through the Jira UI to be valid. So for now, it's necessary for the contributor to reattach the final patch manually (and grant then) before the committer commits it.

          Any suggestions on this one?

          Show
          John Sichi added a comment - @Liyin: yeah, for now we haven't thought of a good solution for the ASF grant yet. I'm not sure but I'm guessing it would need to be done explicitly through the Jira UI to be valid. So for now, it's necessary for the contributor to reattach the final patch manually (and grant then) before the committer commits it. Any suggestions on this one?
          Hide
          John Sichi added a comment -
          Show
          John Sichi added a comment - We've published the JIRA integration module on github: https://github.com/facebook/arc-jira Here's how we made it installable from Hive: https://cwiki.apache.org/confluence/display/Hive/PhabricatorCodeReview https://issues.apache.org/jira/secure/attachment/12500986/D51.2.patch
          Hide
          stack added a comment -

          Whats left on this? A few of us are starting to use this tool. Do we need to port some of John's Hive doc to our manual or at least pointers?

          Show
          stack added a comment - Whats left on this? A few of us are starting to use this tool. Do we need to port some of John's Hive doc to our manual or at least pointers?
          Hide
          John Sichi added a comment -

          Marek has just checked in support for svn workflows as well.

          Show
          John Sichi added a comment - Marek has just checked in support for svn workflows as well.
          Hide
          Nicolas Spiegelberg added a comment -

          need to add a arc-setup option to get the latest arc client changes. working on this now. also need to add documentation for how to setup 'arc' on your system.

          Show
          Nicolas Spiegelberg added a comment - need to add a arc-setup option to get the latest arc client changes. working on this now. also need to add documentation for how to setup 'arc' on your system.
          Hide
          Phabricator added a comment -

          nspiegelberg requested code review of "[jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: stack, JIRA

          Add maven task to setup an arc client & provide a good initial
          default configuration.

          TEST PLAN

          • mvn initialize -Darc

          REVISION DETAIL
          https://reviews.facebook.net/D153

          AFFECTED FILES
          .arcconfig
          .gitignore
          pom.xml

          Show
          Phabricator added a comment - nspiegelberg requested code review of " [jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool". Reviewers: stack, JIRA Add maven task to setup an arc client & provide a good initial default configuration. TEST PLAN mvn initialize -Darc REVISION DETAIL https://reviews.facebook.net/D153 AFFECTED FILES .arcconfig .gitignore pom.xml
          Hide
          Nicolas Spiegelberg added a comment -

          After we checkin D153, we need to make a help wiki similar to Hive's

          https://cwiki.apache.org/confluence/display/Hive/PhabricatorCodeReview

          The main difference is that 'ant arc-setup' becomes 'mvn initialize -Darc'. Note that arc is not a required tool. You can create a diff by uploading a raw diff file ( https://reviews.facebook.net/differential/diff/create/ ). 'arc' is just a command line tool that we use to get stuff done faster. Currently, only 'arc diff' will show you the context around your diff, which is super-nice. Working with Marek to get feature parity here on patch upload.

          Show
          Nicolas Spiegelberg added a comment - After we checkin D153, we need to make a help wiki similar to Hive's https://cwiki.apache.org/confluence/display/Hive/PhabricatorCodeReview The main difference is that 'ant arc-setup' becomes 'mvn initialize -Darc'. Note that arc is not a required tool. You can create a diff by uploading a raw diff file ( https://reviews.facebook.net/differential/diff/create/ ). 'arc' is just a command line tool that we use to get stuff done faster. Currently, only 'arc diff' will show you the context around your diff, which is super-nice. Working with Marek to get feature parity here on patch upload.
          Hide
          Phabricator added a comment -

          nspiegelberg has abandoned the revision "[jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D21

          Show
          Phabricator added a comment - nspiegelberg has abandoned the revision " [jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D21
          Hide
          Phabricator added a comment -

          nspiegelberg has abandoned the revision "[jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D21

          Show
          Phabricator added a comment - nspiegelberg has abandoned the revision " [jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D21
          Hide
          Marek Sapota added a comment -

          It is possible to get context with the web interface, just upload a diff file generated with all the context that you need (for example `git diff -U 9999`. This is exactly what Arcanist does for you with `arc diff`.

          Show
          Marek Sapota added a comment - It is possible to get context with the web interface, just upload a diff file generated with all the context that you need (for example `git diff -U 9999`. This is exactly what Arcanist does for you with `arc diff`.
          Hide
          Phabricator added a comment -

          mareksapotafb has commented on the revision "[jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool".

          INLINE COMMENTS
          .arcconfig:11 Do you really need that there? That just would complain about errors in Arc-JIRA code when you really want to lint HBase code.

          REVISION DETAIL
          https://reviews.facebook.net/D153

          Show
          Phabricator added a comment - mareksapotafb has commented on the revision " [jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool". INLINE COMMENTS .arcconfig:11 Do you really need that there? That just would complain about errors in Arc-JIRA code when you really want to lint HBase code. REVISION DETAIL https://reviews.facebook.net/D153
          Hide
          Phabricator added a comment -

          nspiegelberg has commented on the revision "[jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool".

          INLINE COMMENTS
          .arcconfig:11 I was just copy-pasting a config example you had. I'll remove this.

          REVISION DETAIL
          https://reviews.facebook.net/D153

          Show
          Phabricator added a comment - nspiegelberg has commented on the revision " [jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool". INLINE COMMENTS .arcconfig:11 I was just copy-pasting a config example you had. I'll remove this. REVISION DETAIL https://reviews.facebook.net/D153
          Hide
          Phabricator added a comment -

          jgray has accepted the revision "[jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool".

          lgtm

          REVISION DETAIL
          https://reviews.facebook.net/D153

          Show
          Phabricator added a comment - jgray has accepted the revision " [jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool". lgtm REVISION DETAIL https://reviews.facebook.net/D153
          Hide
          Phabricator added a comment -

          Karthik requested code review of "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA

          Test change

          From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook."

          It's open source so pretty much anyone could host an instance of this software.

          To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org).

          We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D165

          AFFECTED FILES
          README.txt

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/339/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - Karthik requested code review of " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA Test change From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook." It's open source so pretty much anyone could host an instance of this software. To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org ). We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D165 AFFECTED FILES README.txt MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/339/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Phabricator added a comment -

          jgray requested code review of "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA

          Whoo

          From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook."

          It's open source so pretty much anyone could host an instance of this software.

          To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org).

          We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D177

          AFFECTED FILES
          CHANGES.txt

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/351/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - jgray requested code review of " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA Whoo From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook." It's open source so pretty much anyone could host an instance of this software. To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org ). We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D177 AFFECTED FILES CHANGES.txt MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/351/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Phabricator added a comment -

          nspiegelberg has committed the revision "[jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D153

          COMMIT
          https://reviews.facebook.net/rHBASE1196257

          Show
          Phabricator added a comment - nspiegelberg has committed the revision " [jira] HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D153 COMMIT https://reviews.facebook.net/rHBASE1196257
          Hide
          Phabricator added a comment -

          mareksapotafb requested code review of "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA

          abc

          From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook."

          It's open source so pretty much anyone could host an instance of this software.

          To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org).

          We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D171

          AFFECTED FILES
          README.txt

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/345/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - mareksapotafb requested code review of " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA abc From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook." It's open source so pretty much anyone could host an instance of this software. To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org ). We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D171 AFFECTED FILES README.txt MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/345/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Phabricator added a comment -

          jgray requested code review of "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA

          From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook."

          It's open source so pretty much anyone could host an instance of this software.

          To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org).

          We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D183

          AFFECTED FILES
          CHANGES.txt

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/363/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - jgray requested code review of " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook." It's open source so pretty much anyone could host an instance of this software. To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org ). We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D183 AFFECTED FILES CHANGES.txt MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/363/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Phabricator added a comment -

          jgray updated the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA

          REVISION DETAIL
          https://reviews.facebook.net/D177

          AFFECTED FILES
          CHANGES.txt

          Show
          Phabricator added a comment - jgray updated the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA REVISION DETAIL https://reviews.facebook.net/D177 AFFECTED FILES CHANGES.txt
          Hide
          Phabricator added a comment -

          madhuvaidya requested code review of "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA

          Test.

          From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook."

          It's open source so pretty much anyone could host an instance of this software.

          To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org).

          We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D189

          AFFECTED FILES
          README.txt

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/369/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - madhuvaidya requested code review of " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA Test. From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook." It's open source so pretty much anyone could host an instance of this software. To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org ). We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D189 AFFECTED FILES README.txt MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/369/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Phabricator added a comment -

          Karthik updated the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA, madhuvaidya, Kannan

          Testing updating the diff

          REVISION DETAIL
          https://reviews.facebook.net/D165

          AFFECTED FILES
          README.txt

          Show
          Phabricator added a comment - Karthik updated the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA, madhuvaidya, Kannan Testing updating the diff REVISION DETAIL https://reviews.facebook.net/D165 AFFECTED FILES README.txt
          Hide
          Phabricator added a comment -

          Karthik has accepted the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D189

          Show
          Phabricator added a comment - Karthik has accepted the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D189
          Hide
          Phabricator added a comment -

          pritamdamania requested code review of "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA

          1) Changed README.txt

          From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook."

          It's open source so pretty much anyone could host an instance of this software.

          To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org).

          We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D201

          AFFECTED FILES
          README.txt

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/387/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - pritamdamania requested code review of " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA 1) Changed README.txt From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook." It's open source so pretty much anyone could host an instance of this software. To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org ). We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D201 AFFECTED FILES README.txt MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/387/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Phabricator added a comment -

          madhuvaidya has abandoned the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D189

          Show
          Phabricator added a comment - madhuvaidya has abandoned the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D189
          Hide
          Phabricator added a comment -

          Karthik has commented on the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".

          INLINE COMMENTS
          README.txt:31 Need a "9. " here

          REVISION DETAIL
          https://reviews.facebook.net/D189

          Show
          Phabricator added a comment - Karthik has commented on the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". INLINE COMMENTS README.txt:31 Need a "9. " here REVISION DETAIL https://reviews.facebook.net/D189
          Hide
          Phabricator added a comment -

          aaiyer requested code review of "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA

          test diff to see the arc submission process.

          not intended to be committed

          From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook."

          It's open source so pretty much anyone could host an instance of this software.

          To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org).

          We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D207

          AFFECTED FILES
          pom.xml
          README.txt

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/393/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - aaiyer requested code review of " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA test diff to see the arc submission process. not intended to be committed From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook." It's open source so pretty much anyone could host an instance of this software. To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org ). We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D207 AFFECTED FILES pom.xml README.txt MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/393/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Phabricator added a comment -

          mareksapotafb has abandoned the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D171

          Show
          Phabricator added a comment - mareksapotafb has abandoned the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D171
          Hide
          Phabricator added a comment -

          mareksapotafb has abandoned the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D177

          Show
          Phabricator added a comment - mareksapotafb has abandoned the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D177
          Hide
          Phabricator added a comment -

          mareksapotafb has abandoned the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D201

          Show
          Phabricator added a comment - mareksapotafb has abandoned the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D201
          Hide
          Phabricator added a comment -

          mareksapotafb has abandoned the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D165

          Show
          Phabricator added a comment - mareksapotafb has abandoned the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D165
          Hide
          Phabricator added a comment -

          mareksapotafb has abandoned the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D183

          Show
          Phabricator added a comment - mareksapotafb has abandoned the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D183
          Hide
          Phabricator added a comment -

          mareksapotafb has abandoned the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".

          REVISION DETAIL
          https://reviews.facebook.net/D207

          Show
          Phabricator added a comment - mareksapotafb has abandoned the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". REVISION DETAIL https://reviews.facebook.net/D207
          Hide
          Nicolas Spiegelberg added a comment -

          we had the entire FB HBase team go through the steps I mentioned for using arc today. They were able to successfully install arc and use it to create an example diff (hence all the activity on this diff today). Any interested parties, please give it a whirl and give feedback.

          Show
          Nicolas Spiegelberg added a comment - we had the entire FB HBase team go through the steps I mentioned for using arc today. They were able to successfully install arc and use it to create an example diff (hence all the activity on this diff today). Any interested parties, please give it a whirl and give feedback.
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #2397 (See https://builds.apache.org/job/HBase-TRUNK/2397/)
          Fixed CHANGES file for HBASE-4532 & HBASE-4611
          HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool

          nspiegelberg :
          Files :

          • /hbase/trunk/CHANGES.txt

          nspiegelberg :
          Files :

          • /hbase/trunk/.arcconfig
          • /hbase/trunk/.gitignore
          • /hbase/trunk/pom.xml
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #2397 (See https://builds.apache.org/job/HBase-TRUNK/2397/ ) Fixed CHANGES file for HBASE-4532 & HBASE-4611 HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool nspiegelberg : Files : /hbase/trunk/CHANGES.txt nspiegelberg : Files : /hbase/trunk/.arcconfig /hbase/trunk/.gitignore /hbase/trunk/pom.xml
          Hide
          Hudson added a comment -

          Integrated in HBase-0.92 #96 (See https://builds.apache.org/job/HBase-0.92/96/)
          HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool

          nspiegelberg :
          Files :

          • /hbase/branches/0.92/.arcconfig
          • /hbase/branches/0.92/.gitignore
          • /hbase/branches/0.92/CHANGES.txt
          • /hbase/branches/0.92/pom.xml
          Show
          Hudson added a comment - Integrated in HBase-0.92 #96 (See https://builds.apache.org/job/HBase-0.92/96/ ) HBASE-4611 Add support for Phabricator/Differential as an alternative code review tool nspiegelberg : Files : /hbase/branches/0.92/.arcconfig /hbase/branches/0.92/.gitignore /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/pom.xml
          Hide
          Phabricator added a comment -

          nspiegelberg requested code review of "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA

          this is a test

          From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook."

          It's open source so pretty much anyone could host an instance of this software.

          To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org).

          We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D423

          AFFECTED FILES
          bin/hbase

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/843/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - nspiegelberg requested code review of " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA this is a test From http://phabricator.org/ : "Phabricator is a open source collection of web applications which make it easier to write, review, and share source code. It is currently available as an early release. Phabricator was developed at Facebook." It's open source so pretty much anyone could host an instance of this software. To begin with, there will be a public-facing instance located at http://reviews.facebook.net (sponsored by Facebook and hosted by the OSUOSL http://osuosl.org ). We will use this JIRA to deal with adding (and ensuring) Apache-friendly support that will allow us to do code reviews with Phabricator for HBase. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D423 AFFECTED FILES bin/hbase MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/843/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Phabricator added a comment -

          nspiegelberg updated the revision "HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool".
          Reviewers: JIRA

          changed to test2

          REVISION DETAIL
          https://reviews.facebook.net/D423

          AFFECTED FILES
          bin/hbase

          Show
          Phabricator added a comment - nspiegelberg updated the revision " HBASE-4611 [jira] Add support for Phabricator/Differential as an alternative code review tool". Reviewers: JIRA changed to test2 REVISION DETAIL https://reviews.facebook.net/D423 AFFECTED FILES bin/hbase
          Hide
          stack added a comment -

          Do I need a cert to talk to phabricator?

          h-25-20:arcanist stack$ ./bin/arc patch D459
          Usage Exception: YOU NEED TO INSTALL A CERTIFICATE TO LOGIN TO PHABRICATOR
          
          You are trying to connect to 'https://secure.phabricator.com/api/' but do not have a certificate installed for this host. Run:
          
                $ arc install-certificate
          
          ...to install one.
          
          Show
          stack added a comment - Do I need a cert to talk to phabricator? h-25-20:arcanist stack$ ./bin/arc patch D459 Usage Exception: YOU NEED TO INSTALL A CERTIFICATE TO LOGIN TO PHABRICATOR You are trying to connect to 'https: //secure.phabricator.com/api/' but do not have a certificate installed for this host. Run: $ arc install-certificate ...to install one.
          Hide
          Marek Sapota added a comment -

          secure.phabricator.com doesn't allow anonymous access, reviews.facebook.net that we use has anonymous access enabled. Keep in mind that this is a new feature and not everything is "anonymous aware". `arc patch` will work with an updated Arcanist, if you need any other particular workflow to work without a certificate and it doesn't right now, drop me an email, I can probably fix it.

          Show
          Marek Sapota added a comment - secure.phabricator.com doesn't allow anonymous access, reviews.facebook.net that we use has anonymous access enabled. Keep in mind that this is a new feature and not everything is "anonymous aware". `arc patch` will work with an updated Arcanist, if you need any other particular workflow to work without a certificate and it doesn't right now, drop me an email, I can probably fix it.
          Hide
          stack added a comment -

          Thank you Marek. Let me try again later today...

          Show
          stack added a comment - Thank you Marek. Let me try again later today...
          Hide
          Nicolas Spiegelberg added a comment -

          @stack: note that the certificate installation is not a long process. It just has you visit a webpage while logged into FB & give a displayed ID. Although annonymous access for contributors should be fine, I'd recommend secure access for all committers since they'll be approving diffs for inclusion.

          Show
          Nicolas Spiegelberg added a comment - @stack: note that the certificate installation is not a long process. It just has you visit a webpage while logged into FB & give a displayed ID. Although annonymous access for contributors should be fine, I'd recommend secure access for all committers since they'll be approving diffs for inclusion.
          Hide
          Marek Sapota added a comment -

          Anonymous access only allows you read-only actions like browsing the revisions or getting a patch from Differential. Contributors will still need to log in to run `arc diff` or comment on revisions. Also I would call it authenticated access (Phabricator knows who you are, which allows you to upload diffs, etc. as you without manually authenticating every time you run `arc diff`), not secure access to avoid confusion - installing the certificate won't make you more secure and using the anonymous access won't make you more prone to attacks. Both access methods are protected by HTTPS, so Arcanist will talk only to the real reviews.facebook.net and the communication is encrypted.

          Show
          Marek Sapota added a comment - Anonymous access only allows you read-only actions like browsing the revisions or getting a patch from Differential. Contributors will still need to log in to run `arc diff` or comment on revisions. Also I would call it authenticated access (Phabricator knows who you are, which allows you to upload diffs, etc. as you without manually authenticating every time you run `arc diff`), not secure access to avoid confusion - installing the certificate won't make you more secure and using the anonymous access won't make you more prone to attacks. Both access methods are protected by HTTPS, so Arcanist will talk only to the real reviews.facebook.net and the communication is encrypted.
          Hide
          Nicolas Spiegelberg added a comment -

          Fixed. Please see HBASE-4896 to make sure we add proper documentation.

          Show
          Nicolas Spiegelberg added a comment - Fixed. Please see HBASE-4896 to make sure we add proper documentation.

            People

            • Assignee:
              Nicolas Spiegelberg
              Reporter:
              Jonathan Gray
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development