RFC: Phabricator for patches and code review

Hello all, Recently, while doing server maintenance, several of the administrators for Haskell.org set up an instance of Phabricator[1], located at https://phabricator.haskell.org For those who aren't aware, Phabricator (or "Phab") is a suite of tools for software development. Think of it like a polished, semi-private GitHub with a lot of applications and tools for all kinds of needs. We've been using it to do issue tracking for Haskell.org maintenance and like it a lot so far. One very nice aspect of Phabricator though is it has a very nice code review tool, called 'Differential', that is very useful. For people who have used a tool like Review Board, it's similar. Furthermore, it has a very convenient userland tool called 'Arcanist' which makes it easy for newcomers to post a review and get it merged when it's ready all from the command line. I'd like to see if people are interested in using Phab _strictly_ for code review of GHC patches. It is a dedicated tool specifically for this, and I think it works much better than Trac or inline GitHub comments. Also, Phab can also support post-commit reviews. So if I touch something in the runtime system and just push, perhaps Simon or Edward would like to look, and they can be alerted right when I do this, and then yell if I did something stupid. Before I go much further, I'd like to ask: is there *any* interest in this? Or are people satisifed with Trac? The primary motivations are roughly, in no particular order: 1) Code review is good for everyone, a good way for people to learn the code and ask questions, and useful to give feedback to newcomers. And even experienced GHC hackers can learn things from reading code, as we all do regularly, or find things that need cleanup. 2) Phabricator in particular makes it very easy to submit patches for review. To submit a patch, I just run the command 'arc diff' and it Does The Right Thing. It also makes it easy to ensure people are *alerted* when a patch might be relevant to them. 3) They can be uploaded and created from the command line, and merged easily afterwords the same way. This is particularly useful for newcomers, and for me. :) 4) Differential is dedicated to code review, and much better at it than just reading patches on Trac IMO. 5) It supports both post-commit code review, as well as pre-commit review. Post commit would be especially useful for us too, I think. Point #2 and #3 are mostly relevant for me, because I mostly handle incoming patches. But I think in general it would be nice, and make it a lot easier for newcomers to submit patches, and us to look over them. Here's an example of a Differential code review: https://phabricator.haskell.org/D4 This is a demo using my 'wip/ermsb' patch. You'll need to create an account to login, but it shouldn't be much trouble, you can login several ways. I'll fix the login requirement soon. Feel free to read the code, comment on it, and play around. It's more of a demonstration, but real code review would be welcome too. :) If people are interested in doing this, I can add notes to the wiki pages for newcomers, and I'll send another email about Phab so people can understand it a little better. But I want to ask first. There is an argument that our team is so small, code review has unnecessary burdens. But I think Phab could help a lot with tracking outside patches and getting good reviews for incoming patches, and it'll make it easier for newcomers. And experienced pros can probably learn a thing as well. Again, to be clear, I don't propose we migrate anything to Phabricator from, say, Trac. There's no real pressure to do so and it would be tons of work. I only propose we use it for code review, which is perfectly fine, and how other projects like LLVM do code review (they use Bugzilla). I also don't think the usage of Phabricator should be mandatory (unless we decide that later because we like it), but I would like to see people use it if possible. [1] http://phabricator.org -- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

while i'm a novice at using ANY code review tools, having some persistent
tooling for patch reviews would be really great! theres a lot of good
feedback that otherwise only exists in an email somewhere!
On Fri, Jun 6, 2014 at 12:05 AM, Austin Seipp
Hello all,
Recently, while doing server maintenance, several of the administrators for Haskell.org set up an instance of Phabricator[1], located at https://phabricator.haskell.org
For those who aren't aware, Phabricator (or "Phab") is a suite of tools for software development. Think of it like a polished, semi-private GitHub with a lot of applications and tools for all kinds of needs. We've been using it to do issue tracking for Haskell.org maintenance and like it a lot so far.
One very nice aspect of Phabricator though is it has a very nice code review tool, called 'Differential', that is very useful. For people who have used a tool like Review Board, it's similar. Furthermore, it has a very convenient userland tool called 'Arcanist' which makes it easy for newcomers to post a review and get it merged when it's ready all from the command line.
I'd like to see if people are interested in using Phab _strictly_ for code review of GHC patches. It is a dedicated tool specifically for this, and I think it works much better than Trac or inline GitHub comments.
Also, Phab can also support post-commit reviews. So if I touch something in the runtime system and just push, perhaps Simon or Edward would like to look, and they can be alerted right when I do this, and then yell if I did something stupid.
Before I go much further, I'd like to ask: is there *any* interest in this? Or are people satisifed with Trac? The primary motivations are roughly, in no particular order:
1) Code review is good for everyone, a good way for people to learn the code and ask questions, and useful to give feedback to newcomers. And even experienced GHC hackers can learn things from reading code, as we all do regularly, or find things that need cleanup.
2) Phabricator in particular makes it very easy to submit patches for review. To submit a patch, I just run the command 'arc diff' and it Does The Right Thing. It also makes it easy to ensure people are *alerted* when a patch might be relevant to them.
3) They can be uploaded and created from the command line, and merged easily afterwords the same way. This is particularly useful for newcomers, and for me. :)
4) Differential is dedicated to code review, and much better at it than just reading patches on Trac IMO.
5) It supports both post-commit code review, as well as pre-commit review. Post commit would be especially useful for us too, I think.
Point #2 and #3 are mostly relevant for me, because I mostly handle incoming patches. But I think in general it would be nice, and make it a lot easier for newcomers to submit patches, and us to look over them.
Here's an example of a Differential code review:
https://phabricator.haskell.org/D4
This is a demo using my 'wip/ermsb' patch. You'll need to create an account to login, but it shouldn't be much trouble, you can login several ways. I'll fix the login requirement soon. Feel free to read the code, comment on it, and play around. It's more of a demonstration, but real code review would be welcome too. :)
If people are interested in doing this, I can add notes to the wiki pages for newcomers, and I'll send another email about Phab so people can understand it a little better. But I want to ask first.
There is an argument that our team is so small, code review has unnecessary burdens. But I think Phab could help a lot with tracking outside patches and getting good reviews for incoming patches, and it'll make it easier for newcomers. And experienced pros can probably learn a thing as well.
Again, to be clear, I don't propose we migrate anything to Phabricator from, say, Trac. There's no real pressure to do so and it would be tons of work. I only propose we use it for code review, which is perfectly fine, and how other projects like LLVM do code review (they use Bugzilla).
I also don't think the usage of Phabricator should be mandatory (unless we decide that later because we like it), but I would like to see people use it if possible.
-- Regards,
Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/ _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author. So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around? Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well. I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging. Simon | -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs

On Fri, Jun 6, 2014 at 1:28 AM, Simon Peyton Jones
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Yes, that's right - I think it will really help the whole workflow. As for Phabricator: it is used by several very large companies, including Facebook (so Simon might feel at home at least ;), it is actively developed by a team, and the Haskell.org admin team wants to keep using it. I don't think there are any plans for it to go away at this point, but it's still new. There are some other projects, like Review Board, but that leads me to...
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
Good news: the workflow is quite easy at first, you can submit patches from the command line, and yes, the command line tool works on Windows! I *specifically* wanted this when choosing it. I can write up some documents discussing the basic idea. At my last job, we fiddled endlessly with Review Board for basic stuff like "create a review from the commit I just made". Being able to submit and interate patches so quickly is a huge, massive time saver for everyone.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Right. Actually, by default, when you merge in a set of changes using the Phabricator tool (which is mostly what I will do), it squashes them into a single commit, and adds a message pointing back to the review. So the commit is self contained, logical, and small, but you still get the full breadcrumbs, at least.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

So, why not put everything on GutHub and use pull requests and so on?
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

Could not have agreed more with Manuel. I would also like to point out that one of the missions of the arcanist tool is to support all version control systems. That have made sense for FaceBook Inc, who went from Subversion to Git to Mercurial. GHC team only use git now. I think the consequence is that the arcanist command line tool becomes quite weak*, for example I were not able to push a given gitrevision, you have to go through `arc diff` which only pushes the commit that HEAD is at. For sure github is the lead thing most everyone is using and already know how to use. As for side-by-side diffs on github, there is a browser extension for it. But yes, the Phabricator has a better review tool. :) Cheers, Arash * Based on my experience with it from my summer internship at FaceBook 2013. On 2014-06-07 07:21, Manuel M T Chakravarty wrote:
So, why not put everything on GutHub and use pull requests and so on?
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
: At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

I don't think Arcanist forces any particular workflow after working
with it a bit. Generally, all you have to do is checkout a branch,
make some commits on that branch, and run 'arc diff'. Make more
commits on that branch, run 'arc diff' again. When it's ready I can
merge it however I want. This workflow should be very sensible to most
people, and it's easier than uploading diffs to Trac IMO.
Phabricator will not host the repository. Existing committers will
have the exact same access they always did. The only thing that would
change is people submitting patches can use Phabricator instead of
uploading diffs manually.
In addition, existing committers can also use Phabricator's
post-review tools to track commits they might find relevant after
people author them. This is a very useful feature in my experience,
and GitHub has no equivalent to this.
On Sat, Jun 7, 2014 at 2:18 AM, Arash Rouhani
Could not have agreed more with Manuel.
I would also like to point out that one of the missions of the arcanist tool is to support all version control systems. That have made sense for FaceBook Inc, who went from Subversion to Git to Mercurial. GHC team only use git now. I think the consequence is that the arcanist command line tool becomes quite weak*, for example I were not able to push a given gitrevision, you have to go through `arc diff` which only pushes the commit that HEAD is at.
For sure github is the lead thing most everyone is using and already know how to use. As for side-by-side diffs on github, there is a browser extension for it. But yes, the Phabricator has a better review tool. :)
Cheers, Arash
* Based on my experience with it from my summer internship at FaceBook 2013.
On 2014-06-07 07:21, Manuel M T Chakravarty wrote:
So, why not put everything on GutHub and use pull requests and so on?
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
: At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

On 06/07/2014 07:21 AM, Manuel M T Chakravarty wrote:
So, why not put everything on GutHub and use pull requests and so on?
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
While I'm usually in favour of using GitHub for something due to the sheer amount of exposure to people with existing accounts, I have to say that there are few major deficiencies with it for this particular task: issue tracking is crap, you can do very little in ways of customisation, the TOS is pretty scary, labels are not sufficient enough for anything serious, it's very easy to miss comments on specific lines…. It's really not suited for medium/large scale issue tracking and code review. I do agree with you on the 3 disadvantages of using something like Phabricator (and I add an extra one that now a browser so for example you can't use it on X-less box although I'm not sure what the locally installed software does) but if people reading the patches (Austin, Herbert, anyone concerned at the moment) find it easier to do their job AND it becomes easier to ask for feedback then I think we should give it a go. As it stands, I'd rather use e-mail + few policies for code review &c rather than GitHub which is really not suited for this task. Big downside of e-mail/Trac for this is it's hard to give iterative feedback on changing parts of patches.
Simon Peyton Jones
: At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Mateusz K.

This always gets brought up, but I (still) think there are several
reasons to prefer our own infrastructure over GitHub:
- Phab is far more flexible, especially for review. GitHub doesn't
even have side-by-side diffs (a massive improvement), much less the
suite of tools that make code review easy. I cannot set emails to
notify me when someone touches something I am the owner of, for
example, it's just blind emails all the time. This applies to incoming
patches (pre-commit review), and changes merged to the tree
(post-commit review). And both of these workflows are useful ones for
GHC I think.
- I also now think it is *easier* to submit changes for review with
Phabricator having done it, because there is a simple command line
utility to do so. This utility can easily be integrated into the GHC
tree. It is also easier for me to manage patches, or for anyone to
merge patches! It's just one more command.
And the reality is that most commits need to be validated before we
commit them. This has been the standard for a long time - GitHub's
"automated workflows" don't accommodate this. I would rarely, if ever,
hit the "merge button" on GitHub for GHC for example. You must run the
tests for foreign patches you are incorporating - always.
- I don't think maintenance is an issue. We've been using it for
Haskell.org ticket tracking since we needed a replacement for some of
our old infrastructure (including private support tickets which GitHub
does not support), since we're consolidating it all. The admin team
has about 3 people working on it (including me), and we've been doing
upgrades, migrating things, automating them, and generally making the
servers more redundant and simpler.
- I think code review, in general, massively increases the 'shared
knowledge' pool for developers, and a dedicated tool really, really
helps. I can only point to my experience introducing dedicated tools
multiple times in my career in the past as examples. Most people
besides me don't really review patches unless I ask them to, and a
good tool of pending things that can notify you if you might be
interested is really useful.
As for GitHub and Trac in general:
- GitHub lacks several things we already use. For example, there is
no way to add commit hooks to repositories that ban commits containing
whitespace, trailing spaces, and other 'lint' errors. git.haskell.org
automatically enforces this to help keep new code tab-free. GitHub has
no alternative to this.
- We also use this facility to keep submodules sane: as of today,
git.haskell.org will not let you commit a 'dangling submodule
reference' to ghc.git. You must push the corresponding submodule code
first, so the top-level repository never breaks. This is also not
possible with GitHub and has been a historical error source for
developers.
- Speaking of builds, any kind of integration with a CI system, at
some level, is going to require custom infrastructure on our side,
GitHub or not, so there's no clear advantage here. Travis-CI is simply
not going to be long-term acceptable for GHC - we are within 5m or so
of the build limit, and it only builds GHC with some conservative
settings. Also Travis-CI does not allow custom infrastructure, like
ARM buildbots, or multiple differently-versioned OS X buildbots.
Gabor's nightly servers for example allow all of this. Joachim's
builds are very useful though, but we need more.
- Speaking of that, we need to maintain our own server so that Git
pushes can interface with Trac, and the mailing notifier.
- And then you might ask, well we could just migrate it all to
GitHub. That's a _huge_ amount of work. GHC is probably one of the
largest Trac installations around at nearly 10,000 tickets, a gigantic
wiki, and tons of metadata and a *lot* of users. Preserving the
necessary metadata, rewriting intra-wiki links, references, and
preserving everything is just going to be a ton of work. GitHub
doesn't even have an 'import' facility for example, just the raw API,
so this would involve a lot of local processing to 'fix' everything.
This includes moving labels, etc.
I strictly shot down that idea, because it's time I don't think
anyone wants to dedicate to it. Dropping all the data is out of the
question. Trac is well understood at least and has many valuable
aspects, and a migration should be considered a very, very serious
matter after a lot more discussion. Phabricator would only be used for
GHC code review.
On Sat, Jun 7, 2014 at 12:21 AM, Manuel M T Chakravarty
So, why not put everything on GutHub and use pull requests and so on?
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
: At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

Well, I'm convinced now that you have researched this thoroughly. Thanks for doing so and addressing mine and Manuel's concerns. Cheers, Arash On 2014-06-07 09:24, Austin Seipp wrote:
This always gets brought up, but I (still) think there are several reasons to prefer our own infrastructure over GitHub:
- Phab is far more flexible, especially for review. GitHub doesn't even have side-by-side diffs (a massive improvement), much less the suite of tools that make code review easy. I cannot set emails to notify me when someone touches something I am the owner of, for example, it's just blind emails all the time. This applies to incoming patches (pre-commit review), and changes merged to the tree (post-commit review). And both of these workflows are useful ones for GHC I think.
- I also now think it is *easier* to submit changes for review with Phabricator having done it, because there is a simple command line utility to do so. This utility can easily be integrated into the GHC tree. It is also easier for me to manage patches, or for anyone to merge patches! It's just one more command.
And the reality is that most commits need to be validated before we commit them. This has been the standard for a long time - GitHub's "automated workflows" don't accommodate this. I would rarely, if ever, hit the "merge button" on GitHub for GHC for example. You must run the tests for foreign patches you are incorporating - always.
- I don't think maintenance is an issue. We've been using it for Haskell.org ticket tracking since we needed a replacement for some of our old infrastructure (including private support tickets which GitHub does not support), since we're consolidating it all. The admin team has about 3 people working on it (including me), and we've been doing upgrades, migrating things, automating them, and generally making the servers more redundant and simpler.
- I think code review, in general, massively increases the 'shared knowledge' pool for developers, and a dedicated tool really, really helps. I can only point to my experience introducing dedicated tools multiple times in my career in the past as examples. Most people besides me don't really review patches unless I ask them to, and a good tool of pending things that can notify you if you might be interested is really useful.
As for GitHub and Trac in general:
- GitHub lacks several things we already use. For example, there is no way to add commit hooks to repositories that ban commits containing whitespace, trailing spaces, and other 'lint' errors. git.haskell.org automatically enforces this to help keep new code tab-free. GitHub has no alternative to this.
- We also use this facility to keep submodules sane: as of today, git.haskell.org will not let you commit a 'dangling submodule reference' to ghc.git. You must push the corresponding submodule code first, so the top-level repository never breaks. This is also not possible with GitHub and has been a historical error source for developers.
- Speaking of builds, any kind of integration with a CI system, at some level, is going to require custom infrastructure on our side, GitHub or not, so there's no clear advantage here. Travis-CI is simply not going to be long-term acceptable for GHC - we are within 5m or so of the build limit, and it only builds GHC with some conservative settings. Also Travis-CI does not allow custom infrastructure, like ARM buildbots, or multiple differently-versioned OS X buildbots. Gabor's nightly servers for example allow all of this. Joachim's builds are very useful though, but we need more.
- Speaking of that, we need to maintain our own server so that Git pushes can interface with Trac, and the mailing notifier.
- And then you might ask, well we could just migrate it all to GitHub. That's a _huge_ amount of work. GHC is probably one of the largest Trac installations around at nearly 10,000 tickets, a gigantic wiki, and tons of metadata and a *lot* of users. Preserving the necessary metadata, rewriting intra-wiki links, references, and preserving everything is just going to be a ton of work. GitHub doesn't even have an 'import' facility for example, just the raw API, so this would involve a lot of local processing to 'fix' everything. This includes moving labels, etc.
I strictly shot down that idea, because it's time I don't think anyone wants to dedicate to it. Dropping all the data is out of the question. Trac is well understood at least and has many valuable aspects, and a migration should be considered a very, very serious matter after a lot more discussion. Phabricator would only be used for GHC code review.
On Sat, Jun 7, 2014 at 12:21 AM, Manuel M T Chakravarty
wrote: So, why not put everything on GutHub and use pull requests and so on?
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
: At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

On 07/06/2014 07:21, Manuel M T Chakravarty wrote:
So, why not put everything on GutHub and use pull requests and so on?
github just isn't great for doing code reviews. No side-by-side diffs, and it sends you a separate email for every single comment, there's no concept of a "review" consisting of multiple inline comments (unless I've missed something). I'm afraid if we were using this for regular reviews I would have to disable the email notifications, which makes it significantly less useful.
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
Well, you've tipped the balance with "somewhat" and "significant" here, I'd say Phabricator is "significantly" better than github for code reviews, while installing arc is "somewhat" annoying :-) I have to admit it's not a no-brainer, but I do worry that github just wouldn't cut it for doing a lot of code reviewing, whereas I spend my life inside Phabricator so I know it works really well. What's more, github doesn't let you put animated gifs in code reviews. Need I say more? Cheers, Simon
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
: At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

It seems that most people are in favour of using Phabricator for code review. So what are the next steps? Can we just start using the existing phabricator instance? I'm working on some code right now that definitely needs reviewing. Janek Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał:
On 07/06/2014 07:21, Manuel M T Chakravarty wrote:
So, why not put everything on GutHub and use pull requests and so on?
github just isn't great for doing code reviews. No side-by-side diffs, and it sends you a separate email for every single comment, there's no concept of a "review" consisting of multiple inline comments (unless I've missed something). I'm afraid if we were using this for regular reviews I would have to disable the email notifications, which makes it significantly less useful.
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
Well, you've tipped the balance with "somewhat" and "significant" here, I'd say Phabricator is "significantly" better than github for code reviews, while installing arc is "somewhat" annoying :-)
I have to admit it's not a no-brainer, but I do worry that github just wouldn't cut it for doing a lot of code reviewing, whereas I spend my life inside Phabricator so I know it works really well.
What's more, github doesn't let you put animated gifs in code reviews. Need I say more?
Cheers, Simon
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
: At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of | Austin Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the | administrators for Haskell.org set up an instance of Phabricator[1], | located at https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of | tools for software development. Think of it like a polished, | semi-private GitHub with a lot of applications and tools for all kinds | of needs. We've been using it to do issue tracking for Haskell.org | maintenance and like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people | who have used a tool like Review Board, it's similar. Furthermore, it | has a very convenient userland tool called 'Arcanist' which makes it | easy for newcomers to post a review and get it merged when it's ready | all from the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch | something in the runtime system and just push, perhaps Simon or Edward | would like to look, and they can be alerted right when I do this, and | then yell if I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn | the code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, | as we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it | Does The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it | than just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it | a lot easier for newcomers to submit patches, and us to look over | them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read | the code, comment on it, and play around. It's more of a | demonstration, but real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and | it'll make it easier for newcomers. And experienced pros can probably | learn a thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be | tons of work. I only propose we use it for code review, which is | perfectly fine, and how other projects like LLVM do code review (they | use Bugzilla). | | I also don't think the usage of Phabricator should be mandatory | (unless we decide that later because we like it), but I would like to | see people use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

On 13/06/14 10:47, Jan Stolarek wrote:
It seems that most people are in favour of using Phabricator for code review. So what are the next steps? Can we just start using the existing phabricator instance? I'm working on some code right now that definitely needs reviewing.
You can use it, and a few of us have already been doing so. There isn't any Trac integration yet, but it works nicely for patch review. There's a short intro doc here: https://secure.phabricator.com/book/phabricator/article/differential/, but it's not hard to figure out the basics, and you'll learn by watching how other people use it. If you go to the Herald tool you have yourself automatically subscribed to diffs that touch areas of the code that you're interested in. Pro tip: the keyboard shortcuts are really useful, especially "z". Hit "?" to see all the shortcuts. Cheers, Simon
Janek
Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał:
On 07/06/2014 07:21, Manuel M T Chakravarty wrote:
So, why not put everything on GutHub and use pull requests and so on?
github just isn't great for doing code reviews. No side-by-side diffs, and it sends you a separate email for every single comment, there's no concept of a "review" consisting of multiple inline comments (unless I've missed something). I'm afraid if we were using this for regular reviews I would have to disable the email notifications, which makes it significantly less useful.
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
Well, you've tipped the balance with "somewhat" and "significant" here, I'd say Phabricator is "significantly" better than github for code reviews, while installing arc is "somewhat" annoying :-)
I have to admit it's not a no-brainer, but I do worry that github just wouldn't cut it for doing a lot of code reviewing, whereas I spend my life inside Phabricator so I know it works really well.
What's more, github doesn't let you put animated gifs in code reviews. Need I say more?
Cheers, Simon
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
: At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of | Austin Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the | administrators for Haskell.org set up an instance of Phabricator[1], | located at https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of | tools for software development. Think of it like a polished, | semi-private GitHub with a lot of applications and tools for all kinds | of needs. We've been using it to do issue tracking for Haskell.org | maintenance and like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people | who have used a tool like Review Board, it's similar. Furthermore, it | has a very convenient userland tool called 'Arcanist' which makes it | easy for newcomers to post a review and get it merged when it's ready | all from the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch | something in the runtime system and just push, perhaps Simon or Edward | would like to look, and they can be alerted right when I do this, and | then yell if I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn | the code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, | as we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it | Does The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it | than just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it | a lot easier for newcomers to submit patches, and us to look over | them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read | the code, comment on it, and play around. It's more of a | demonstration, but real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and | it'll make it easier for newcomers. And experienced pros can probably | learn a thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be | tons of work. I only propose we use it for code review, which is | perfectly fine, and how other projects like LLVM do code review (they | use Bugzilla). | | I also don't think the usage of Phabricator should be mandatory | (unless we decide that later because we like it), but I would like to | see people use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

You can use it, and a few of us have already been doing so. There isn't any Trac integration yet, but it works nicely for patch review. Right. I was wondering about the inclusion of phabricator utilities in the GHC tree - I believe this was mentioned in the discussion.
There's a short intro doc here: https://secure.phabricator.com/book/phabricator/article/differential/, but it's not hard to figure out the basics, and you'll learn by watching how other people use it. No doubt, although I must say that it doesn't yet feel intuitive and some UI things could be done better.
If you go to the Herald tool you have yourself automatically subscribed to diffs that touch areas of the code that you're interested in. Is there a way to create a notification for a particular directory in the GHC source tree? I see I can create a notification for "Any changed filename contains ..." - if I give a part of path (say compiler/basicTypes) will I get notifications if anything in that directory changes?
Pro tip: the keyboard shortcuts are really useful, especially "z". Hit "?" to see all the shortcuts. Duh, no C-x prefix ;-)
Janek

I read the friendly Arcanist manual and I wonder if we intend to have a default .arcconfig file in the GHC repo? From the docs it seems like a good idea. Janek Dnia wtorek, 17 czerwca 2014, Simon Marlow napisał:
On 13/06/14 10:47, Jan Stolarek wrote:
It seems that most people are in favour of using Phabricator for code review. So what are the next steps? Can we just start using the existing phabricator instance? I'm working on some code right now that definitely needs reviewing.
You can use it, and a few of us have already been doing so. There isn't any Trac integration yet, but it works nicely for patch review.
There's a short intro doc here: https://secure.phabricator.com/book/phabricator/article/differential/, but it's not hard to figure out the basics, and you'll learn by watching how other people use it. If you go to the Herald tool you have yourself automatically subscribed to diffs that touch areas of the code that you're interested in.
Pro tip: the keyboard shortcuts are really useful, especially "z". Hit "?" to see all the shortcuts.
Cheers, Simon
Janek
Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał:
On 07/06/2014 07:21, Manuel M T Chakravarty wrote:
So, why not put everything on GutHub and use pull requests and so on?
github just isn't great for doing code reviews. No side-by-side diffs, and it sends you a separate email for every single comment, there's no concept of a "review" consisting of multiple inline comments (unless I've missed something). I'm afraid if we were using this for regular reviews I would have to disable the email notifications, which makes it significantly less useful.
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
Well, you've tipped the balance with "somewhat" and "significant" here, I'd say Phabricator is "significantly" better than github for code reviews, while installing arc is "somewhat" annoying :-)
I have to admit it's not a no-brainer, but I do worry that github just wouldn't cut it for doing a lot of code reviewing, whereas I spend my life inside Phabricator so I know it works really well.
What's more, github doesn't let you put animated gifs in code reviews. Need I say more?
Cheers, Simon
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
: At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of | Austin Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the | administrators for Haskell.org set up an instance of Phabricator[1], | located at https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of | tools for software development. Think of it like a polished, | semi-private GitHub with a lot of applications and tools for all | kinds of needs. We've been using it to do issue tracking for | Haskell.org maintenance and like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice | code review tool, called 'Differential', that is very useful. For | people who have used a tool like Review Board, it's similar. | Furthermore, it has a very convenient userland tool called | 'Arcanist' which makes it easy for newcomers to post a review and | get it merged when it's ready all from the command line. | | I'd like to see if people are interested in using Phab _strictly_ | for code review of GHC patches. It is a dedicated tool specifically | for this, and I think it works much better than Trac or inline | GitHub comments. | | Also, Phab can also support post-commit reviews. So if I touch | something in the runtime system and just push, perhaps Simon or | Edward would like to look, and they can be alerted right when I do | this, and then yell if I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest | in this? Or are people satisifed with Trac? The primary motivations | are roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn | the code and ask questions, and useful to give feedback to | newcomers. And even experienced GHC hackers can learn things from | reading code, as we all do regularly, or find things that need | cleanup. | | 2) Phabricator in particular makes it very easy to submit patches | for review. To submit a patch, I just run the command 'arc diff' and | it Does The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and | merged easily afterwords the same way. This is particularly useful | for newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it | than just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make | it a lot easier for newcomers to submit patches, and us to look over | them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read | the code, comment on it, and play around. It's more of a | demonstration, but real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so | people can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and | it'll make it easier for newcomers. And experienced pros can | probably learn a thing as well. | | Again, to be clear, I don't propose we migrate anything to | Phabricator from, say, Trac. There's no real pressure to do so and | it would be tons of work. I only propose we use it for code review, | which is perfectly fine, and how other projects like LLVM do code | review (they use Bugzilla). | | I also don't think the usage of Phabricator should be mandatory | (unless we decide that later because we like it), but I would like | to see people use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

Duh, ignore what I wrote. I just realized I'm working on a non-rebased branch :-) Janek Dnia środa, 18 czerwca 2014, Jan Stolarek napisał:
I read the friendly Arcanist manual and I wonder if we intend to have a default .arcconfig file in the GHC repo? From the docs it seems like a good idea.
Janek
Dnia wtorek, 17 czerwca 2014, Simon Marlow napisał:
On 13/06/14 10:47, Jan Stolarek wrote:
It seems that most people are in favour of using Phabricator for code review. So what are the next steps? Can we just start using the existing phabricator instance? I'm working on some code right now that definitely needs reviewing.
You can use it, and a few of us have already been doing so. There isn't any Trac integration yet, but it works nicely for patch review.
There's a short intro doc here: https://secure.phabricator.com/book/phabricator/article/differential/, but it's not hard to figure out the basics, and you'll learn by watching how other people use it. If you go to the Herald tool you have yourself automatically subscribed to diffs that touch areas of the code that you're interested in.
Pro tip: the keyboard shortcuts are really useful, especially "z". Hit "?" to see all the shortcuts.
Cheers, Simon
Janek
Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał:
On 07/06/2014 07:21, Manuel M T Chakravarty wrote:
So, why not put everything on GutHub and use pull requests and so on?
github just isn't great for doing code reviews. No side-by-side diffs, and it sends you a separate email for every single comment, there's no concept of a "review" consisting of multiple inline comments (unless I've missed something). I'm afraid if we were using this for regular reviews I would have to disable the email notifications, which makes it significantly less useful.
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
Well, you've tipped the balance with "somewhat" and "significant" here, I'd say Phabricator is "significantly" better than github for code reviews, while installing arc is "somewhat" annoying :-)
I have to admit it's not a no-brainer, but I do worry that github just wouldn't cut it for doing a lot of code reviewing, whereas I spend my life inside Phabricator so I know it works really well.
What's more, github doesn't let you put animated gifs in code reviews. Need I say more?
Cheers, Simon
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
: At the moment GHC's main sources aren't on github, which means that that (in my highly imperfect understanding) people can't submit pull requests or use their code review mechanisms. Moreover, most people don't have commit rights on the main GHC server, so if someone wants to offer a patch they can really only do so in textual form attached to Trac. People with commit rights can make a branch, but there's a danger that over a decade we'll accumulate zillions of dead branches which people forgot to delete. I think on github the branch is in a different repo, belonging to the patch author.
So we really don't have a good work flow for creating, reviewing, modifying, and finally apply patches. I am no expert on these matters. If Phabricator would help with that I'm all for it. But perhaps there are other alternatives? Or is Phab the lead thing. Will it stay around?
Also before going too far I'd really like someone to document the workflow carefully, and make sure it works from Windows equally well.
I'm not too stressed out about losing the review trail of a patch. Much of it will be commenting on stuff that no longer appears in the final patch. Anything that's important should appear in a Note in the source code; even the commit messages are invisible until you really start digging.
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of | Austin Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the | administrators for Haskell.org set up an instance of | Phabricator[1], located at https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of | tools for software development. Think of it like a polished, | semi-private GitHub with a lot of applications and tools for all | kinds of needs. We've been using it to do issue tracking for | Haskell.org maintenance and like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice | code review tool, called 'Differential', that is very useful. For | people who have used a tool like Review Board, it's similar. | Furthermore, it has a very convenient userland tool called | 'Arcanist' which makes it easy for newcomers to post a review and | get it merged when it's ready all from the command line. | | I'd like to see if people are interested in using Phab _strictly_ | for code review of GHC patches. It is a dedicated tool | specifically for this, and I think it works much better than Trac | or inline GitHub comments. | | Also, Phab can also support post-commit reviews. So if I touch | something in the runtime system and just push, perhaps Simon or | Edward would like to look, and they can be alerted right when I do | this, and then yell if I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest | in this? Or are people satisifed with Trac? The primary | motivations are roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to | learn the code and ask questions, and useful to give feedback to | newcomers. And even experienced GHC hackers can learn things from | reading code, as we all do regularly, or find things that need | cleanup. | | 2) Phabricator in particular makes it very easy to submit patches | for review. To submit a patch, I just run the command 'arc diff' | and it Does The Right Thing. It also makes it easy to ensure | people are *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and | merged easily afterwords the same way. This is particularly useful | for newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at | it than just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as | pre-commit review. Post commit would be especially useful for us | too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly | handle incoming patches. But I think in general it would be nice, | and make it a lot easier for newcomers to submit patches, and us | to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create | an account to login, but it shouldn't be much trouble, you can | login several ways. I'll fix the login requirement soon. Feel free | to read the code, comment on it, and play around. It's more of a | demonstration, but real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the | wiki pages for newcomers, and I'll send another email about Phab | so people can understand it a little better. But I want to ask | first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with | tracking outside patches and getting good reviews for incoming | patches, and it'll make it easier for newcomers. And experienced | pros can probably learn a thing as well. | | Again, to be clear, I don't propose we migrate anything to | Phabricator from, say, Trac. There's no real pressure to do so and | it would be tons of work. I only propose we use it for code | review, which is perfectly fine, and how other projects like LLVM | do code review (they use Bugzilla). | | I also don't think the usage of Phabricator should be mandatory | (unless we decide that later because we like it), but I would like | to see people use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

Hi all,
I went ahead and took some time to write some stuff down about
Phabricator, including some basic tips on the workflows and
applications here:
https://ghc.haskell.org/trac/ghc/wiki/Phabricator
It's definitely going to need more expanding. Do let me know if
anything is confusing.
On Wed, Jun 18, 2014 at 2:38 AM, Jan Stolarek
Duh, ignore what I wrote. I just realized I'm working on a non-rebased branch :-)
Janek
Dnia środa, 18 czerwca 2014, Jan Stolarek napisał:
I read the friendly Arcanist manual and I wonder if we intend to have a default .arcconfig file in the GHC repo? From the docs it seems like a good idea.
Janek
Dnia wtorek, 17 czerwca 2014, Simon Marlow napisał:
On 13/06/14 10:47, Jan Stolarek wrote:
It seems that most people are in favour of using Phabricator for code review. So what are the next steps? Can we just start using the existing phabricator instance? I'm working on some code right now that definitely needs reviewing.
You can use it, and a few of us have already been doing so. There isn't any Trac integration yet, but it works nicely for patch review.
There's a short intro doc here: https://secure.phabricator.com/book/phabricator/article/differential/, but it's not hard to figure out the basics, and you'll learn by watching how other people use it. If you go to the Herald tool you have yourself automatically subscribed to diffs that touch areas of the code that you're interested in.
Pro tip: the keyboard shortcuts are really useful, especially "z". Hit "?" to see all the shortcuts.
Cheers, Simon
Janek
Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał:
On 07/06/2014 07:21, Manuel M T Chakravarty wrote:
So, why not put everything on GutHub and use pull requests and so on?
github just isn't great for doing code reviews. No side-by-side diffs, and it sends you a separate email for every single comment, there's no concept of a "review" consisting of multiple inline comments (unless I've missed something). I'm afraid if we were using this for regular reviews I would have to disable the email notifications, which makes it significantly less useful.
SimonM writes that Phabricator is better than GitHub. I’m happy to believe that, but he also writes that using it requires installing local software and quite a bit of work. Moreover, I like to add that lots of people already know how to use GitHub and probably few know Phabricator.
So, we are talking about having a somewhat better tool in return for three very significant disadvantages: (1) local installation, (2) work to set up and maintain Phabricator, and (3) effort by many people to learn to use it.
Well, you've tipped the balance with "somewhat" and "significant" here, I'd say Phabricator is "significantly" better than github for code reviews, while installing arc is "somewhat" annoying :-)
I have to admit it's not a no-brainer, but I do worry that github just wouldn't cut it for doing a lot of code reviewing, whereas I spend my life inside Phabricator so I know it works really well.
What's more, github doesn't let you put animated gifs in code reviews. Need I say more?
Cheers, Simon
We also have a constant lack of sufficient men power. So, why spend effort on building our own infrastructure, which will only increase the hurdle for contributors (as they have to deal with an unknown system)? Let’s outsource the effort to GitHub.
Manuel
Simon Peyton Jones
: > At the moment GHC's main sources aren't on github, which means that > that (in my highly imperfect understanding) people can't submit pull > requests or use their code review mechanisms. Moreover, most people > don't have commit rights on the main GHC server, so if someone wants > to offer a patch they can really only do so in textual form attached > to Trac. People with commit rights can make a branch, but there's a > danger that over a decade we'll accumulate zillions of dead branches > which people forgot to delete. I think on github the branch is in a > different repo, belonging to the patch author. > > So we really don't have a good work flow for creating, reviewing, > modifying, and finally apply patches. I am no expert on these > matters. If Phabricator would help with that I'm all for it. But > perhaps there are other alternatives? Or is Phab the lead thing. > Will it stay around? > > Also before going too far I'd really like someone to document the > workflow carefully, and make sure it works from Windows equally > well. > > I'm not too stressed out about losing the review trail of a patch. > Much of it will be commenting on stuff that no longer appears in the > final patch. Anything that's important should appear in a Note in > the source code; even the commit messages are invisible until you > really start digging. > > Simon > > | -----Original Message----- > | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of > | Austin Seipp > | Sent: 06 June 2014 05:06 > | To: ghc-devs@haskell.org > | Subject: RFC: Phabricator for patches and code review > | > | Hello all, > | > | Recently, while doing server maintenance, several of the > | administrators for Haskell.org set up an instance of > | Phabricator[1], located at https://phabricator.haskell.org > | > | For those who aren't aware, Phabricator (or "Phab") is a suite of > | tools for software development. Think of it like a polished, > | semi-private GitHub with a lot of applications and tools for all > | kinds of needs. We've been using it to do issue tracking for > | Haskell.org maintenance and like it a lot so far. > | > | One very nice aspect of Phabricator though is it has a very nice > | code review tool, called 'Differential', that is very useful. For > | people who have used a tool like Review Board, it's similar. > | Furthermore, it has a very convenient userland tool called > | 'Arcanist' which makes it easy for newcomers to post a review and > | get it merged when it's ready all from the command line. > | > | I'd like to see if people are interested in using Phab _strictly_ > | for code review of GHC patches. It is a dedicated tool > | specifically for this, and I think it works much better than Trac > | or inline GitHub comments. > | > | Also, Phab can also support post-commit reviews. So if I touch > | something in the runtime system and just push, perhaps Simon or > | Edward would like to look, and they can be alerted right when I do > | this, and then yell if I did something stupid. > | > | Before I go much further, I'd like to ask: is there *any* interest > | in this? Or are people satisifed with Trac? The primary > | motivations are roughly, in no particular order: > | > | 1) Code review is good for everyone, a good way for people to > | learn the code and ask questions, and useful to give feedback to > | newcomers. And even experienced GHC hackers can learn things from > | reading code, as we all do regularly, or find things that need > | cleanup. > | > | 2) Phabricator in particular makes it very easy to submit patches > | for review. To submit a patch, I just run the command 'arc diff' > | and it Does The Right Thing. It also makes it easy to ensure > | people are *alerted* when a patch might be relevant to them. > | > | 3) They can be uploaded and created from the command line, and > | merged easily afterwords the same way. This is particularly useful > | for newcomers, and for me. :) > | > | 4) Differential is dedicated to code review, and much better at > | it than just reading patches on Trac IMO. > | > | 5) It supports both post-commit code review, as well as > | pre-commit review. Post commit would be especially useful for us > | too, I think. > | > | Point #2 and #3 are mostly relevant for me, because I mostly > | handle incoming patches. But I think in general it would be nice, > | and make it a lot easier for newcomers to submit patches, and us > | to look over them. > | > | Here's an example of a Differential code review: > | > | https://phabricator.haskell.org/D4 > | > | This is a demo using my 'wip/ermsb' patch. You'll need to create > | an account to login, but it shouldn't be much trouble, you can > | login several ways. I'll fix the login requirement soon. Feel free > | to read the code, comment on it, and play around. It's more of a > | demonstration, but real code review would be welcome too. :) > | > | If people are interested in doing this, I can add notes to the > | wiki pages for newcomers, and I'll send another email about Phab > | so people can understand it a little better. But I want to ask > | first. > | > | There is an argument that our team is so small, code review has > | unnecessary burdens. But I think Phab could help a lot with > | tracking outside patches and getting good reviews for incoming > | patches, and it'll make it easier for newcomers. And experienced > | pros can probably learn a thing as well. > | > | Again, to be clear, I don't propose we migrate anything to > | Phabricator from, say, Trac. There's no real pressure to do so and > | it would be tons of work. I only propose we use it for code > | review, which is perfectly fine, and how other projects like LLVM > | do code review (they use Bugzilla). > | > | I also don't think the usage of Phabricator should be mandatory > | (unless we decide that later because we like it), but I would like > | to see people use it if possible. > | > | [1] http://phabricator.org > | > | -- > | Regards, > | > | Austin Seipp, Haskell Consultant > | Well-Typed LLP, http://www.well-typed.com/ > | _______________________________________________ > | ghc-devs mailing list > | ghc-devs@haskell.org > | http://www.haskell.org/mailman/listinfo/ghc-devs > > _______________________________________________ > ghc-devs mailing list > ghc-devs@haskell.org > http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

Thanks so much for writing this! I have some questions:
1) I'm just setting things up on my machine. It says to `arc install-certificate` in my GHC directory. Is it important precisely which clone of GHC my directory is set up against? For example, my "pull" origin is git://git.haskell.org/ghc.git and my "push" origin is ssh://git@git.haskell.org/ghc.git. Is this the right set-up? If this bit matters, could you add it to the page? Or, if not, could you comment on what `arc` is pulling from the ghc directory?
2) I'm confused about what, precisely, `arc diff` does. You describe that it updates the review available online. Does that reference some git commits? Do I need to push by `wip` branch before `arc diff`ing? Do I *never* need to push my branch, because `arc diff` pushes it for me? Do I *never* need to push my branch, because Phab uses other ways of moving the code around? For better or worse, I tend to keep my branches local until I'm ready to merge with master, and I want to know if this needs to change.
3) You say "A change cannot be merged until at least one reviewer has signed off." Does this mean "merged with ghc-7.8" (or whatever the current stable branch is)? Or does it mean "merged with master"? The former is the status quo, but with a new route, so to speak. The latter is something new, as several of us push directly to `master` without a review. I'm not against such a change, per se, just trying to understand it.
4) Is it now compulsory to use Phab when contributing? This page makes it sound that way. Again, no complaints -- just trying to understand it.
5) The page says to add `austin` as a reviewer. I would expect `thoughtpolice`. What's up with Phab usernames? Do other people I know and love have Phab usernames distinct from Trac usernames?
6) Who is the reviewer `#ghc`? Is this related to the IRC channel? What does the # signify?
7) I'm baffled by "Landing reviews". The `arc land` bit is clearer, but I'm still unsure of what my local state and the git upstream state must be beforehand for this to work. Will this ask me for a commit message? Will it use the one I specified to Phab during `arc diff`? In general, I'm confused about how much info `arc` pulls from various places to do its work. I know I could learn by doing, but I'm afraid of mashing on the Phab and/or git history as I do so.
8) Some of the same questions surround `arc patch`: What does my git state need to be for this to work?
9) How do I contribute to others' revisions? Or, will this be obvious what it comes to pass?
I realize I've asked a lot here, and it might be too much to expect all of these answers to be on the page. If that's the case, could you perhaps link to relevant manuals or places to learn more? The way `arc` works, in particular, seems like magic; magic is very powerful, but it can be equally dangerous, and so I'd like to learn more.
Thanks so much for doing all this!
Richard
On Jun 23, 2014, at 10:44 AM, Austin Seipp
Hi all,
I went ahead and took some time to write some stuff down about Phabricator, including some basic tips on the workflows and applications here:
https://ghc.haskell.org/trac/ghc/wiki/Phabricator
It's definitely going to need more expanding. Do let me know if anything is confusing.
On Wed, Jun 18, 2014 at 2:38 AM, Jan Stolarek
wrote: Duh, ignore what I wrote. I just realized I'm working on a non-rebased branch :-)
Janek
Dnia środa, 18 czerwca 2014, Jan Stolarek napisał:
I read the friendly Arcanist manual and I wonder if we intend to have a default .arcconfig file in the GHC repo? From the docs it seems like a good idea.
Janek
Dnia wtorek, 17 czerwca 2014, Simon Marlow napisał:
On 13/06/14 10:47, Jan Stolarek wrote:
It seems that most people are in favour of using Phabricator for code review. So what are the next steps? Can we just start using the existing phabricator instance? I'm working on some code right now that definitely needs reviewing.
You can use it, and a few of us have already been doing so. There isn't any Trac integration yet, but it works nicely for patch review.
There's a short intro doc here: https://secure.phabricator.com/book/phabricator/article/differential/, but it's not hard to figure out the basics, and you'll learn by watching how other people use it. If you go to the Herald tool you have yourself automatically subscribed to diffs that touch areas of the code that you're interested in.
Pro tip: the keyboard shortcuts are really useful, especially "z". Hit "?" to see all the shortcuts.
Cheers, Simon
Janek
Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał:
On 07/06/2014 07:21, Manuel M T Chakravarty wrote: > So, why not put everything on GutHub and use pull requests and so on?
github just isn't great for doing code reviews. No side-by-side diffs, and it sends you a separate email for every single comment, there's no concept of a "review" consisting of multiple inline comments (unless I've missed something). I'm afraid if we were using this for regular reviews I would have to disable the email notifications, which makes it significantly less useful.
> SimonM writes that Phabricator is better than GitHub. I’m happy to > believe that, but he also writes that using it requires installing > local software and quite a bit of work. Moreover, I like to add that > lots of people already know how to use GitHub and probably few know > Phabricator. > > So, we are talking about having a somewhat better tool in return for > three very significant disadvantages: (1) local installation, (2) > work to set up and maintain Phabricator, and (3) effort by many > people to learn to use it.
Well, you've tipped the balance with "somewhat" and "significant" here, I'd say Phabricator is "significantly" better than github for code reviews, while installing arc is "somewhat" annoying :-)
I have to admit it's not a no-brainer, but I do worry that github just wouldn't cut it for doing a lot of code reviewing, whereas I spend my life inside Phabricator so I know it works really well.
What's more, github doesn't let you put animated gifs in code reviews. Need I say more?
Cheers, Simon
> We also have a constant lack of sufficient men power. So, why spend > effort on building our own infrastructure, which will only increase > the hurdle for contributors (as they have to deal with an unknown > system)? Let’s outsource the effort to GitHub. > > Manuel > > Simon Peyton Jones
: >> At the moment GHC's main sources aren't on github, which means that >> that (in my highly imperfect understanding) people can't submit pull >> requests or use their code review mechanisms. Moreover, most people >> don't have commit rights on the main GHC server, so if someone wants >> to offer a patch they can really only do so in textual form attached >> to Trac. People with commit rights can make a branch, but there's a >> danger that over a decade we'll accumulate zillions of dead branches >> which people forgot to delete. I think on github the branch is in a >> different repo, belonging to the patch author. >> >> So we really don't have a good work flow for creating, reviewing, >> modifying, and finally apply patches. I am no expert on these >> matters. If Phabricator would help with that I'm all for it. But >> perhaps there are other alternatives? Or is Phab the lead thing. >> Will it stay around? >> >> Also before going too far I'd really like someone to document the >> workflow carefully, and make sure it works from Windows equally >> well. >> >> I'm not too stressed out about losing the review trail of a patch. >> Much of it will be commenting on stuff that no longer appears in the >> final patch. Anything that's important should appear in a Note in >> the source code; even the commit messages are invisible until you >> really start digging. >> >> Simon >> >> | -----Original Message----- >> | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of >> | Austin Seipp >> | Sent: 06 June 2014 05:06 >> | To: ghc-devs@haskell.org >> | Subject: RFC: Phabricator for patches and code review >> | >> | Hello all, >> | >> | Recently, while doing server maintenance, several of the >> | administrators for Haskell.org set up an instance of >> | Phabricator[1], located at https://phabricator.haskell.org >> | >> | For those who aren't aware, Phabricator (or "Phab") is a suite of >> | tools for software development. Think of it like a polished, >> | semi-private GitHub with a lot of applications and tools for all >> | kinds of needs. We've been using it to do issue tracking for >> | Haskell.org maintenance and like it a lot so far. >> | >> | One very nice aspect of Phabricator though is it has a very nice >> | code review tool, called 'Differential', that is very useful. For >> | people who have used a tool like Review Board, it's similar. >> | Furthermore, it has a very convenient userland tool called >> | 'Arcanist' which makes it easy for newcomers to post a review and >> | get it merged when it's ready all from the command line. >> | >> | I'd like to see if people are interested in using Phab _strictly_ >> | for code review of GHC patches. It is a dedicated tool >> | specifically for this, and I think it works much better than Trac >> | or inline GitHub comments. >> | >> | Also, Phab can also support post-commit reviews. So if I touch >> | something in the runtime system and just push, perhaps Simon or >> | Edward would like to look, and they can be alerted right when I do >> | this, and then yell if I did something stupid. >> | >> | Before I go much further, I'd like to ask: is there *any* interest >> | in this? Or are people satisifed with Trac? The primary >> | motivations are roughly, in no particular order: >> | >> | 1) Code review is good for everyone, a good way for people to >> | learn the code and ask questions, and useful to give feedback to >> | newcomers. And even experienced GHC hackers can learn things from >> | reading code, as we all do regularly, or find things that need >> | cleanup. >> | >> | 2) Phabricator in particular makes it very easy to submit patches >> | for review. To submit a patch, I just run the command 'arc diff' >> | and it Does The Right Thing. It also makes it easy to ensure >> | people are *alerted* when a patch might be relevant to them. >> | >> | 3) They can be uploaded and created from the command line, and >> | merged easily afterwords the same way. This is particularly useful >> | for newcomers, and for me. :) >> | >> | 4) Differential is dedicated to code review, and much better at >> | it than just reading patches on Trac IMO. >> | >> | 5) It supports both post-commit code review, as well as >> | pre-commit review. Post commit would be especially useful for us >> | too, I think. >> | >> | Point #2 and #3 are mostly relevant for me, because I mostly >> | handle incoming patches. But I think in general it would be nice, >> | and make it a lot easier for newcomers to submit patches, and us >> | to look over them. >> | >> | Here's an example of a Differential code review: >> | >> | https://phabricator.haskell.org/D4 >> | >> | This is a demo using my 'wip/ermsb' patch. You'll need to create >> | an account to login, but it shouldn't be much trouble, you can >> | login several ways. I'll fix the login requirement soon. Feel free >> | to read the code, comment on it, and play around. It's more of a >> | demonstration, but real code review would be welcome too. :) >> | >> | If people are interested in doing this, I can add notes to the >> | wiki pages for newcomers, and I'll send another email about Phab >> | so people can understand it a little better. But I want to ask >> | first. >> | >> | There is an argument that our team is so small, code review has >> | unnecessary burdens. But I think Phab could help a lot with >> | tracking outside patches and getting good reviews for incoming >> | patches, and it'll make it easier for newcomers. And experienced >> | pros can probably learn a thing as well. >> | >> | Again, to be clear, I don't propose we migrate anything to >> | Phabricator from, say, Trac. There's no real pressure to do so and >> | it would be tons of work. I only propose we use it for code >> | review, which is perfectly fine, and how other projects like LLVM >> | do code review (they use Bugzilla). >> | >> | I also don't think the usage of Phabricator should be mandatory >> | (unless we decide that later because we like it), but I would like >> | to see people use it if possible. >> | >> | [1] http://phabricator.org >> | >> | -- >> | Regards, >> | >> | Austin Seipp, Haskell Consultant >> | Well-Typed LLP, http://www.well-typed.com/ >> | _______________________________________________ >> | ghc-devs mailing list >> | ghc-devs@haskell.org >> | http://www.haskell.org/mailman/listinfo/ghc-devs >> >> _______________________________________________ >> ghc-devs mailing list >> ghc-devs@haskell.org >> http://www.haskell.org/mailman/listinfo/ghc-devs > > _______________________________________________ > ghc-devs mailing list > ghc-devs@haskell.org > http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards,
Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/ _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

I find the automatic squashing to rather harmful to the commit history. So
if you have several nice commits that you want to send for review, don't
use arc land to commit them, as it will ruin the history. Instead git push
them as per normal and use `arc close` (IIRC) to close the code review. I
also find it useful to remove lots of the arc gunk that was added to the
commit message, to not pollute our revision history with data from some
specific tool.
On Tue, Jun 24, 2014 at 4:09 PM, Richard Eisenberg
Thanks so much for writing this! I have some questions:
1) I'm just setting things up on my machine. It says to `arc install-certificate` in my GHC directory. Is it important precisely which clone of GHC my directory is set up against? For example, my "pull" origin is git://git.haskell.org/ghc.git and my "push" origin is ssh:// git@git.haskell.org/ghc.git. Is this the right set-up? If this bit matters, could you add it to the page? Or, if not, could you comment on what `arc` is pulling from the ghc directory?
2) I'm confused about what, precisely, `arc diff` does. You describe that it updates the review available online. Does that reference some git commits? Do I need to push by `wip` branch before `arc diff`ing? Do I *never* need to push my branch, because `arc diff` pushes it for me? Do I *never* need to push my branch, because Phab uses other ways of moving the code around? For better or worse, I tend to keep my branches local until I'm ready to merge with master, and I want to know if this needs to change.
3) You say "A change cannot be merged until at least one reviewer has signed off." Does this mean "merged with ghc-7.8" (or whatever the current stable branch is)? Or does it mean "merged with master"? The former is the status quo, but with a new route, so to speak. The latter is something new, as several of us push directly to `master` without a review. I'm not against such a change, per se, just trying to understand it.
4) Is it now compulsory to use Phab when contributing? This page makes it sound that way. Again, no complaints -- just trying to understand it.
5) The page says to add `austin` as a reviewer. I would expect `thoughtpolice`. What's up with Phab usernames? Do other people I know and love have Phab usernames distinct from Trac usernames?
6) Who is the reviewer `#ghc`? Is this related to the IRC channel? What does the # signify?
7) I'm baffled by "Landing reviews". The `arc land` bit is clearer, but I'm still unsure of what my local state and the git upstream state must be beforehand for this to work. Will this ask me for a commit message? Will it use the one I specified to Phab during `arc diff`? In general, I'm confused about how much info `arc` pulls from various places to do its work. I know I could learn by doing, but I'm afraid of mashing on the Phab and/or git history as I do so.
8) Some of the same questions surround `arc patch`: What does my git state need to be for this to work?
9) How do I contribute to others' revisions? Or, will this be obvious what it comes to pass?
I realize I've asked a lot here, and it might be too much to expect all of these answers to be on the page. If that's the case, could you perhaps link to relevant manuals or places to learn more? The way `arc` works, in particular, seems like magic; magic is very powerful, but it can be equally dangerous, and so I'd like to learn more.
Thanks so much for doing all this! Richard
On Jun 23, 2014, at 10:44 AM, Austin Seipp
wrote: Hi all,
I went ahead and took some time to write some stuff down about Phabricator, including some basic tips on the workflows and applications here:
https://ghc.haskell.org/trac/ghc/wiki/Phabricator
It's definitely going to need more expanding. Do let me know if anything is confusing.
Duh, ignore what I wrote. I just realized I'm working on a non-rebased branch :-)
Janek
Dnia środa, 18 czerwca 2014, Jan Stolarek napisał:
I read the friendly Arcanist manual and I wonder if we intend to have a default .arcconfig file in the GHC repo? From the docs it seems like a good idea.
Janek
Dnia wtorek, 17 czerwca 2014, Simon Marlow napisał:
On 13/06/14 10:47, Jan Stolarek wrote:
It seems that most people are in favour of using Phabricator for code review. So what are the next steps? Can we just start using the existing phabricator instance? I'm working on some code right now
definitely needs reviewing.
You can use it, and a few of us have already been doing so. There isn't any Trac integration yet, but it works nicely for patch review.
There's a short intro doc here: https://secure.phabricator.com/book/phabricator/article/differential/ , but it's not hard to figure out the basics, and you'll learn by watching how other people use it. If you go to the Herald tool you have yourself automatically subscribed to diffs that touch areas of the code that you're interested in.
Pro tip: the keyboard shortcuts are really useful, especially "z". Hit "?" to see all the shortcuts.
Cheers, Simon
Janek
Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał: > On 07/06/2014 07:21, Manuel M T Chakravarty wrote: >> So, why not put everything on GutHub and use pull requests and so on? > > github just isn't great for doing code reviews. No side-by-side diffs, > and it sends you a separate email for every single comment, there's no > concept of a "review" consisting of multiple inline comments (unless > I've missed something). I'm afraid if we were using this for regular > reviews I would have to disable the email notifications, which makes > it significantly less useful. > >> SimonM writes that Phabricator is better than GitHub. I’m happy to >> believe that, but he also writes that using it requires installing >> local software and quite a bit of work. Moreover, I like to add
>> lots of people already know how to use GitHub and probably few know >> Phabricator. >> >> So, we are talking about having a somewhat better tool in return for >> three very significant disadvantages: (1) local installation, (2) >> work to set up and maintain Phabricator, and (3) effort by many >> people to learn to use it. > > Well, you've tipped the balance with "somewhat" and "significant" > here, I'd say Phabricator is "significantly" better than github for > code reviews, while installing arc is "somewhat" annoying :-) > > I have to admit it's not a no-brainer, but I do worry that github just > wouldn't cut it for doing a lot of code reviewing, whereas I spend my > life inside Phabricator so I know it works really well. > > What's more, github doesn't let you put animated gifs in code reviews. > Need I say more? > > Cheers, > Simon > >> We also have a constant lack of sufficient men power. So, why spend >> effort on building our own infrastructure, which will only increase >> the hurdle for contributors (as they have to deal with an unknown >> system)? Let’s outsource the effort to GitHub. >> >> Manuel >> >> Simon Peyton Jones
: >>> At the moment GHC's main sources aren't on github, which means >>> that (in my highly imperfect understanding) people can't submit
>>> requests or use their code review mechanisms. Moreover, most
>>> don't have commit rights on the main GHC server, so if someone wants >>> to offer a patch they can really only do so in textual form attached >>> to Trac. People with commit rights can make a branch, but there's a >>> danger that over a decade we'll accumulate zillions of dead branches >>> which people forgot to delete. I think on github the branch is in a >>> different repo, belonging to the patch author. >>> >>> So we really don't have a good work flow for creating, reviewing, >>> modifying, and finally apply patches. I am no expert on these >>> matters. If Phabricator would help with that I'm all for it. But >>> perhaps there are other alternatives? Or is Phab the lead thing. >>> Will it stay around? >>> >>> Also before going too far I'd really like someone to document the >>> workflow carefully, and make sure it works from Windows equally >>> well. >>> >>> I'm not too stressed out about losing the review trail of a patch. >>> Much of it will be commenting on stuff that no longer appears in
>>> final patch. Anything that's important should appear in a Note in >>> the source code; even the commit messages are invisible until you >>> really start digging. >>> >>> Simon >>> >>> | -----Original Message----- >>> | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of >>> | Austin Seipp >>> | Sent: 06 June 2014 05:06 >>> | To: ghc-devs@haskell.org >>> | Subject: RFC: Phabricator for patches and code review >>> | >>> | Hello all, >>> | >>> | Recently, while doing server maintenance, several of the >>> | administrators for Haskell.org set up an instance of >>> | Phabricator[1], located at https://phabricator.haskell.org >>> | >>> | For those who aren't aware, Phabricator (or "Phab") is a suite of >>> | tools for software development. Think of it like a polished, >>> | semi-private GitHub with a lot of applications and tools for all >>> | kinds of needs. We've been using it to do issue tracking for >>> | Haskell.org maintenance and like it a lot so far. >>> | >>> | One very nice aspect of Phabricator though is it has a very nice >>> | code review tool, called 'Differential', that is very useful. For >>> | people who have used a tool like Review Board, it's similar. >>> | Furthermore, it has a very convenient userland tool called >>> | 'Arcanist' which makes it easy for newcomers to post a review and >>> | get it merged when it's ready all from the command line. >>> | >>> | I'd like to see if people are interested in using Phab _strictly_ >>> | for code review of GHC patches. It is a dedicated tool >>> | specifically for this, and I think it works much better than Trac >>> | or inline GitHub comments. >>> | >>> | Also, Phab can also support post-commit reviews. So if I touch >>> | something in the runtime system and just push, perhaps Simon or >>> | Edward would like to look, and they can be alerted right when I do >>> | this, and then yell if I did something stupid. >>> | >>> | Before I go much further, I'd like to ask: is there *any* interest >>> | in this? Or are people satisifed with Trac? The primary >>> | motivations are roughly, in no particular order: >>> | >>> | 1) Code review is good for everyone, a good way for people to >>> | learn the code and ask questions, and useful to give feedback to >>> | newcomers. And even experienced GHC hackers can learn things from >>> | reading code, as we all do regularly, or find things that need >>> | cleanup. >>> | >>> | 2) Phabricator in particular makes it very easy to submit
>>> | for review. To submit a patch, I just run the command 'arc diff' >>> | and it Does The Right Thing. It also makes it easy to ensure >>> | people are *alerted* when a patch might be relevant to them. >>> | >>> | 3) They can be uploaded and created from the command line, and >>> | merged easily afterwords the same way. This is particularly useful >>> | for newcomers, and for me. :) >>> | >>> | 4) Differential is dedicated to code review, and much better at >>> | it than just reading patches on Trac IMO. >>> | >>> | 5) It supports both post-commit code review, as well as >>> | pre-commit review. Post commit would be especially useful for us >>> | too, I think. >>> | >>> | Point #2 and #3 are mostly relevant for me, because I mostly >>> | handle incoming patches. But I think in general it would be nice, >>> | and make it a lot easier for newcomers to submit patches, and us >>> | to look over them. >>> | >>> | Here's an example of a Differential code review: >>> | >>> | https://phabricator.haskell.org/D4 >>> | >>> | This is a demo using my 'wip/ermsb' patch. You'll need to create >>> | an account to login, but it shouldn't be much trouble, you can >>> | login several ways. I'll fix the login requirement soon. Feel free >>> | to read the code, comment on it, and play around. It's more of a >>> | demonstration, but real code review would be welcome too. :) >>> | >>> | If people are interested in doing this, I can add notes to the >>> | wiki pages for newcomers, and I'll send another email about Phab >>> | so people can understand it a little better. But I want to ask >>> | first. >>> | >>> | There is an argument that our team is so small, code review has >>> | unnecessary burdens. But I think Phab could help a lot with >>> | tracking outside patches and getting good reviews for incoming >>> | patches, and it'll make it easier for newcomers. And experienced >>> | pros can probably learn a thing as well. >>> | >>> | Again, to be clear, I don't propose we migrate anything to >>> | Phabricator from, say, Trac. There's no real pressure to do so and >>> | it would be tons of work. I only propose we use it for code >>> | review, which is perfectly fine, and how other projects like LLVM >>> | do code review (they use Bugzilla). >>> | >>> | I also don't think the usage of Phabricator should be mandatory >>> | (unless we decide that later because we like it), but I would
On Wed, Jun 18, 2014 at 2:38 AM, Jan Stolarek
wrote: that that that pull people the patches like >>> | to see people use it if possible. >>> | >>> | [1] http://phabricator.org >>> | >>> | -- >>> | Regards, >>> | >>> | Austin Seipp, Haskell Consultant >>> | Well-Typed LLP, http://www.well-typed.com/ >>> | _______________________________________________ >>> | ghc-devs mailing list >>> | ghc-devs@haskell.org >>> | http://www.haskell.org/mailman/listinfo/ghc-devs >>> >>> _______________________________________________ >>> ghc-devs mailing list >>> ghc-devs@haskell.org >>> http://www.haskell.org/mailman/listinfo/ghc-devs >> >> _______________________________________________ >> ghc-devs mailing list >> ghc-devs@haskell.org >> http://www.haskell.org/mailman/listinfo/ghc-devs > > _______________________________________________ > ghc-devs mailing list > ghc-devs@haskell.org > http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards,
Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/ _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

Sounds plausible to me, but please can this workflow be documented on the wiki page?
From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Johan Tibell
Sent: 24 June 2014 15:27
To: Richard Eisenberg
Cc: ghc-devs@haskell.org
Subject: Re: Phabricator for patches and code review
I find the automatic squashing to rather harmful to the commit history. So if you have several nice commits that you want to send for review, don't use arc land to commit them, as it will ruin the history. Instead git push them as per normal and use `arc close` (IIRC) to close the code review. I also find it useful to remove lots of the arc gunk that was added to the commit message, to not pollute our revision history with data from some specific tool.
On Tue, Jun 24, 2014 at 4:09 PM, Richard Eisenberg
Hi all,
I went ahead and took some time to write some stuff down about Phabricator, including some basic tips on the workflows and applications here:
https://ghc.haskell.org/trac/ghc/wiki/Phabricator
It's definitely going to need more expanding. Do let me know if anything is confusing.
On Wed, Jun 18, 2014 at 2:38 AM, Jan Stolarek
mailto:jan.stolarek@p.lodz.pl> wrote: Duh, ignore what I wrote. I just realized I'm working on a non-rebased branch :-)
Janek
Dnia środa, 18 czerwca 2014, Jan Stolarek napisał:
I read the friendly Arcanist manual and I wonder if we intend to have a default .arcconfig file in the GHC repo? From the docs it seems like a good idea.
Janek
Dnia wtorek, 17 czerwca 2014, Simon Marlow napisał:
On 13/06/14 10:47, Jan Stolarek wrote:
It seems that most people are in favour of using Phabricator for code review. So what are the next steps? Can we just start using the existing phabricator instance? I'm working on some code right now that definitely needs reviewing.
You can use it, and a few of us have already been doing so. There isn't any Trac integration yet, but it works nicely for patch review.
There's a short intro doc here: https://secure.phabricator.com/book/phabricator/article/differential/, but it's not hard to figure out the basics, and you'll learn by watching how other people use it. If you go to the Herald tool you have yourself automatically subscribed to diffs that touch areas of the code that you're interested in.
Pro tip: the keyboard shortcuts are really useful, especially "z". Hit "?" to see all the shortcuts.
Cheers, Simon
Janek
Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał:
On 07/06/2014 07:21, Manuel M T Chakravarty wrote: > So, why not put everything on GutHub and use pull requests and so on?
github just isn't great for doing code reviews. No side-by-side diffs, and it sends you a separate email for every single comment, there's no concept of a "review" consisting of multiple inline comments (unless I've missed something). I'm afraid if we were using this for regular reviews I would have to disable the email notifications, which makes it significantly less useful.
> SimonM writes that Phabricator is better than GitHub. I’m happy to > believe that, but he also writes that using it requires installing > local software and quite a bit of work. Moreover, I like to add that > lots of people already know how to use GitHub and probably few know > Phabricator. > > So, we are talking about having a somewhat better tool in return for > three very significant disadvantages: (1) local installation, (2) > work to set up and maintain Phabricator, and (3) effort by many > people to learn to use it.
Well, you've tipped the balance with "somewhat" and "significant" here, I'd say Phabricator is "significantly" better than github for code reviews, while installing arc is "somewhat" annoying :-)
I have to admit it's not a no-brainer, but I do worry that github just wouldn't cut it for doing a lot of code reviewing, whereas I spend my life inside Phabricator so I know it works really well.
What's more, github doesn't let you put animated gifs in code reviews. Need I say more?
Cheers, Simon
> We also have a constant lack of sufficient men power. So, why spend > effort on building our own infrastructure, which will only increase > the hurdle for contributors (as they have to deal with an unknown > system)? Let’s outsource the effort to GitHub. > > Manuel > > Simon Peyton Jones
mailto:simonpj@microsoft.com>: >> At the moment GHC's main sources aren't on github, which means that >> that (in my highly imperfect understanding) people can't submit pull >> requests or use their code review mechanisms. Moreover, most people >> don't have commit rights on the main GHC server, so if someone wants >> to offer a patch they can really only do so in textual form attached >> to Trac. People with commit rights can make a branch, but there's a >> danger that over a decade we'll accumulate zillions of dead branches >> which people forgot to delete. I think on github the branch is in a >> different repo, belonging to the patch author. >> >> So we really don't have a good work flow for creating, reviewing, >> modifying, and finally apply patches. I am no expert on these >> matters. If Phabricator would help with that I'm all for it. But >> perhaps there are other alternatives? Or is Phab the lead thing. >> Will it stay around? >> >> Also before going too far I'd really like someone to document the >> workflow carefully, and make sure it works from Windows equally >> well. >> >> I'm not too stressed out about losing the review trail of a patch. >> Much of it will be commenting on stuff that no longer appears in the >> final patch. Anything that's important should appear in a Note in >> the source code; even the commit messages are invisible until you >> really start digging. >> >> Simon >> >> | -----Original Message----- >> | From: ghc-devs [mailto:ghc-devs-bounces@haskell.orgmailto:ghc-devs-bounces@haskell.org] On Behalf Of >> | Austin Seipp >> | Sent: 06 June 2014 05:06 >> | To: ghc-devs@haskell.orgmailto:ghc-devs@haskell.org >> | Subject: RFC: Phabricator for patches and code review >> | >> | Hello all, >> | >> | Recently, while doing server maintenance, several of the >> | administrators for Haskell.org set up an instance of >> | Phabricator[1], located at https://phabricator.haskell.org >> | >> | For those who aren't aware, Phabricator (or "Phab") is a suite of >> | tools for software development. Think of it like a polished, >> | semi-private GitHub with a lot of applications and tools for all >> | kinds of needs. We've been using it to do issue tracking for >> | Haskell.org maintenance and like it a lot so far. >> | >> | One very nice aspect of Phabricator though is it has a very nice >> | code review tool, called 'Differential', that is very useful. For >> | people who have used a tool like Review Board, it's similar. >> | Furthermore, it has a very convenient userland tool called >> | 'Arcanist' which makes it easy for newcomers to post a review and >> | get it merged when it's ready all from the command line. >> | >> | I'd like to see if people are interested in using Phab _strictly_ >> | for code review of GHC patches. It is a dedicated tool >> | specifically for this, and I think it works much better than Trac >> | or inline GitHub comments. >> | >> | Also, Phab can also support post-commit reviews. So if I touch >> | something in the runtime system and just push, perhaps Simon or >> | Edward would like to look, and they can be alerted right when I do >> | this, and then yell if I did something stupid. >> | >> | Before I go much further, I'd like to ask: is there *any* interest >> | in this? Or are people satisifed with Trac? The primary >> | motivations are roughly, in no particular order: >> | >> | 1) Code review is good for everyone, a good way for people to >> | learn the code and ask questions, and useful to give feedback to >> | newcomers. And even experienced GHC hackers can learn things from >> | reading code, as we all do regularly, or find things that need >> | cleanup. >> | >> | 2) Phabricator in particular makes it very easy to submit patches >> | for review. To submit a patch, I just run the command 'arc diff' >> | and it Does The Right Thing. It also makes it easy to ensure >> | people are *alerted* when a patch might be relevant to them. >> | >> | 3) They can be uploaded and created from the command line, and >> | merged easily afterwords the same way. This is particularly useful >> | for newcomers, and for me. :) >> | >> | 4) Differential is dedicated to code review, and much better at >> | it than just reading patches on Trac IMO. >> | >> | 5) It supports both post-commit code review, as well as >> | pre-commit review. Post commit would be especially useful for us >> | too, I think. >> | >> | Point #2 and #3 are mostly relevant for me, because I mostly >> | handle incoming patches. But I think in general it would be nice, >> | and make it a lot easier for newcomers to submit patches, and us >> | to look over them. >> | >> | Here's an example of a Differential code review: >> | >> | https://phabricator.haskell.org/D4 >> | >> | This is a demo using my 'wip/ermsb' patch. You'll need to create >> | an account to login, but it shouldn't be much trouble, you can >> | login several ways. I'll fix the login requirement soon. Feel free >> | to read the code, comment on it, and play around. It's more of a >> | demonstration, but real code review would be welcome too. :) >> | >> | If people are interested in doing this, I can add notes to the >> | wiki pages for newcomers, and I'll send another email about Phab >> | so people can understand it a little better. But I want to ask >> | first. >> | >> | There is an argument that our team is so small, code review has >> | unnecessary burdens. But I think Phab could help a lot with >> | tracking outside patches and getting good reviews for incoming >> | patches, and it'll make it easier for newcomers. And experienced >> | pros can probably learn a thing as well. >> | >> | Again, to be clear, I don't propose we migrate anything to >> | Phabricator from, say, Trac. There's no real pressure to do so and >> | it would be tons of work. I only propose we use it for code >> | review, which is perfectly fine, and how other projects like LLVM >> | do code review (they use Bugzilla). >> | >> | I also don't think the usage of Phabricator should be mandatory >> | (unless we decide that later because we like it), but I would like >> | to see people use it if possible. >> | >> | [1] http://phabricator.org >> | >> | -- >> | Regards, >> | >> | Austin Seipp, Haskell Consultant >> | Well-Typed LLP, http://www.well-typed.com/ >> | _______________________________________________ >> | ghc-devs mailing list >> | ghc-devs@haskell.orgmailto:ghc-devs@haskell.org >> | http://www.haskell.org/mailman/listinfo/ghc-devs >> >> _______________________________________________ >> ghc-devs mailing list >> ghc-devs@haskell.orgmailto:ghc-devs@haskell.org >> http://www.haskell.org/mailman/listinfo/ghc-devs > > _______________________________________________ > ghc-devs mailing list > ghc-devs@haskell.orgmailto:ghc-devs@haskell.org > http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.orgmailto:ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.orgmailto:ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.orgmailto:ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards,
Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/ _______________________________________________ ghc-devs mailing list ghc-devs@haskell.orgmailto:ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.orgmailto:ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

Personally I don't particularly mind the fact Phabricator squashes
things and adds onto the URL. For one it means we can always refer to
the differential revision solely by the commit itself, as opposed to
digging up some history. But also, Phab generally asks you to put some
useful information in there. I've found myself to be much more
explicit and verbose when filling out Phab commits than most other
tools, and meaty, verbose commit messages are very useful IME.
As for squashing, there's an argument to be made, I think, that this
just requires a little practice to get comfortable with. :) For
example, I have a review called D4:
https://phabricator.haskell.org/D4
This really adds two separate things: it adds a set of flags to the
frontend, and a set of optimizations to the backend (based on the
flags). Really, this *should* be two reviews instead of one! The fact
that Phabricator wants to squash them into a single commit when
landing is a symptom of the problem, not the root problem, I think. So
while I may have to fiddle to separate the two later (not really that
hard,) I think this was more a case of me putting two unrelated things
in one review.
In fact it's very regular for GHC developers to squash commits
themselves, and I know many of us do exactly that. So I think the main
change is just having the tool embody this for us, as opposed to
always doing it manually.
On Tue, Jun 24, 2014 at 9:27 AM, Johan Tibell
I find the automatic squashing to rather harmful to the commit history. So if you have several nice commits that you want to send for review, don't use arc land to commit them, as it will ruin the history. Instead git push them as per normal and use `arc close` (IIRC) to close the code review. I also find it useful to remove lots of the arc gunk that was added to the commit message, to not pollute our revision history with data from some specific tool.
On Tue, Jun 24, 2014 at 4:09 PM, Richard Eisenberg
wrote: Thanks so much for writing this! I have some questions:
1) I'm just setting things up on my machine. It says to `arc install-certificate` in my GHC directory. Is it important precisely which clone of GHC my directory is set up against? For example, my "pull" origin is git://git.haskell.org/ghc.git and my "push" origin is ssh://git@git.haskell.org/ghc.git. Is this the right set-up? If this bit matters, could you add it to the page? Or, if not, could you comment on what `arc` is pulling from the ghc directory?
2) I'm confused about what, precisely, `arc diff` does. You describe that it updates the review available online. Does that reference some git commits? Do I need to push by `wip` branch before `arc diff`ing? Do I *never* need to push my branch, because `arc diff` pushes it for me? Do I *never* need to push my branch, because Phab uses other ways of moving the code around? For better or worse, I tend to keep my branches local until I'm ready to merge with master, and I want to know if this needs to change.
3) You say "A change cannot be merged until at least one reviewer has signed off." Does this mean "merged with ghc-7.8" (or whatever the current stable branch is)? Or does it mean "merged with master"? The former is the status quo, but with a new route, so to speak. The latter is something new, as several of us push directly to `master` without a review. I'm not against such a change, per se, just trying to understand it.
4) Is it now compulsory to use Phab when contributing? This page makes it sound that way. Again, no complaints -- just trying to understand it.
5) The page says to add `austin` as a reviewer. I would expect `thoughtpolice`. What's up with Phab usernames? Do other people I know and love have Phab usernames distinct from Trac usernames?
6) Who is the reviewer `#ghc`? Is this related to the IRC channel? What does the # signify?
7) I'm baffled by "Landing reviews". The `arc land` bit is clearer, but I'm still unsure of what my local state and the git upstream state must be beforehand for this to work. Will this ask me for a commit message? Will it use the one I specified to Phab during `arc diff`? In general, I'm confused about how much info `arc` pulls from various places to do its work. I know I could learn by doing, but I'm afraid of mashing on the Phab and/or git history as I do so.
8) Some of the same questions surround `arc patch`: What does my git state need to be for this to work?
9) How do I contribute to others' revisions? Or, will this be obvious what it comes to pass?
I realize I've asked a lot here, and it might be too much to expect all of these answers to be on the page. If that's the case, could you perhaps link to relevant manuals or places to learn more? The way `arc` works, in particular, seems like magic; magic is very powerful, but it can be equally dangerous, and so I'd like to learn more.
Thanks so much for doing all this! Richard
On Jun 23, 2014, at 10:44 AM, Austin Seipp
wrote: Hi all,
I went ahead and took some time to write some stuff down about Phabricator, including some basic tips on the workflows and applications here:
https://ghc.haskell.org/trac/ghc/wiki/Phabricator
It's definitely going to need more expanding. Do let me know if anything is confusing.
On Wed, Jun 18, 2014 at 2:38 AM, Jan Stolarek
wrote: Duh, ignore what I wrote. I just realized I'm working on a non-rebased branch :-)
Janek
Dnia środa, 18 czerwca 2014, Jan Stolarek napisał:
I read the friendly Arcanist manual and I wonder if we intend to have a default .arcconfig file in the GHC repo? From the docs it seems like a good idea.
Janek
Dnia wtorek, 17 czerwca 2014, Simon Marlow napisał:
On 13/06/14 10:47, Jan Stolarek wrote: > It seems that most people are in favour of using Phabricator for > code > review. So what are the next steps? Can we just start using the > existing phabricator instance? I'm working on some code right now > that > definitely needs reviewing.
You can use it, and a few of us have already been doing so. There isn't any Trac integration yet, but it works nicely for patch review.
There's a short intro doc here:
https://secure.phabricator.com/book/phabricator/article/differential/, but it's not hard to figure out the basics, and you'll learn by watching how other people use it. If you go to the Herald tool you have yourself automatically subscribed to diffs that touch areas of the code that you're interested in.
Pro tip: the keyboard shortcuts are really useful, especially "z". Hit "?" to see all the shortcuts.
Cheers, Simon
> Janek > > Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał: >> On 07/06/2014 07:21, Manuel M T Chakravarty wrote: >>> So, why not put everything on GutHub and use pull requests and so >>> on? >> >> github just isn't great for doing code reviews. No side-by-side >> diffs, >> and it sends you a separate email for every single comment, there's >> no >> concept of a "review" consisting of multiple inline comments >> (unless >> I've missed something). I'm afraid if we were using this for >> regular >> reviews I would have to disable the email notifications, which >> makes >> it significantly less useful. >> >>> SimonM writes that Phabricator is better than GitHub. I’m happy to >>> believe that, but he also writes that using it requires installing >>> local software and quite a bit of work. Moreover, I like to add >>> that >>> lots of people already know how to use GitHub and probably few >>> know >>> Phabricator. >>> >>> So, we are talking about having a somewhat better tool in return >>> for >>> three very significant disadvantages: (1) local installation, (2) >>> work to set up and maintain Phabricator, and (3) effort by many >>> people to learn to use it. >> >> Well, you've tipped the balance with "somewhat" and "significant" >> here, I'd say Phabricator is "significantly" better than github for >> code reviews, while installing arc is "somewhat" annoying :-) >> >> I have to admit it's not a no-brainer, but I do worry that github >> just >> wouldn't cut it for doing a lot of code reviewing, whereas I spend >> my >> life inside Phabricator so I know it works really well. >> >> What's more, github doesn't let you put animated gifs in code >> reviews. >> Need I say more? >> >> Cheers, >> Simon >> >>> We also have a constant lack of sufficient men power. So, why >>> spend >>> effort on building our own infrastructure, which will only >>> increase >>> the hurdle for contributors (as they have to deal with an unknown >>> system)? Let’s outsource the effort to GitHub. >>> >>> Manuel >>> >>> Simon Peyton Jones
: >>>> At the moment GHC's main sources aren't on github, which means >>>> that >>>> that (in my highly imperfect understanding) people can't submit >>>> pull >>>> requests or use their code review mechanisms. Moreover, most >>>> people >>>> don't have commit rights on the main GHC server, so if someone >>>> wants >>>> to offer a patch they can really only do so in textual form >>>> attached >>>> to Trac. People with commit rights can make a branch, but there's >>>> a >>>> danger that over a decade we'll accumulate zillions of dead >>>> branches >>>> which people forgot to delete. I think on github the branch is >>>> in a >>>> different repo, belonging to the patch author. >>>> >>>> So we really don't have a good work flow for creating, reviewing, >>>> modifying, and finally apply patches. I am no expert on these >>>> matters. If Phabricator would help with that I'm all for it. But >>>> perhaps there are other alternatives? Or is Phab the lead thing. >>>> Will it stay around? >>>> >>>> Also before going too far I'd really like someone to document the >>>> workflow carefully, and make sure it works from Windows equally >>>> well. >>>> >>>> I'm not too stressed out about losing the review trail of a >>>> patch. >>>> Much of it will be commenting on stuff that no longer appears in >>>> the >>>> final patch. Anything that's important should appear in a Note >>>> in >>>> the source code; even the commit messages are invisible until you >>>> really start digging. >>>> >>>> Simon >>>> >>>> | -----Original Message----- >>>> | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf >>>> Of >>>> | Austin Seipp >>>> | Sent: 06 June 2014 05:06 >>>> | To: ghc-devs@haskell.org >>>> | Subject: RFC: Phabricator for patches and code review >>>> | >>>> | Hello all, >>>> | >>>> | Recently, while doing server maintenance, several of the >>>> | administrators for Haskell.org set up an instance of >>>> | Phabricator[1], located at https://phabricator.haskell.org >>>> | >>>> | For those who aren't aware, Phabricator (or "Phab") is a suite >>>> of >>>> | tools for software development. Think of it like a polished, >>>> | semi-private GitHub with a lot of applications and tools for >>>> all >>>> | kinds of needs. We've been using it to do issue tracking for >>>> | Haskell.org maintenance and like it a lot so far. >>>> | >>>> | One very nice aspect of Phabricator though is it has a very >>>> nice >>>> | code review tool, called 'Differential', that is very useful. >>>> For >>>> | people who have used a tool like Review Board, it's similar. >>>> | Furthermore, it has a very convenient userland tool called >>>> | 'Arcanist' which makes it easy for newcomers to post a review >>>> and >>>> | get it merged when it's ready all from the command line. >>>> | >>>> | I'd like to see if people are interested in using Phab >>>> _strictly_ >>>> | for code review of GHC patches. It is a dedicated tool >>>> | specifically for this, and I think it works much better than >>>> Trac >>>> | or inline GitHub comments. >>>> | >>>> | Also, Phab can also support post-commit reviews. So if I touch >>>> | something in the runtime system and just push, perhaps Simon or >>>> | Edward would like to look, and they can be alerted right when I >>>> do >>>> | this, and then yell if I did something stupid. >>>> | >>>> | Before I go much further, I'd like to ask: is there *any* >>>> interest >>>> | in this? Or are people satisifed with Trac? The primary >>>> | motivations are roughly, in no particular order: >>>> | >>>> | 1) Code review is good for everyone, a good way for people to >>>> | learn the code and ask questions, and useful to give feedback >>>> to >>>> | newcomers. And even experienced GHC hackers can learn things >>>> from >>>> | reading code, as we all do regularly, or find things that need >>>> | cleanup. >>>> | >>>> | 2) Phabricator in particular makes it very easy to submit >>>> patches >>>> | for review. To submit a patch, I just run the command 'arc >>>> diff' >>>> | and it Does The Right Thing. It also makes it easy to ensure >>>> | people are *alerted* when a patch might be relevant to them. >>>> | >>>> | 3) They can be uploaded and created from the command line, and >>>> | merged easily afterwords the same way. This is particularly >>>> useful >>>> | for newcomers, and for me. :) >>>> | >>>> | 4) Differential is dedicated to code review, and much better >>>> at >>>> | it than just reading patches on Trac IMO. >>>> | >>>> | 5) It supports both post-commit code review, as well as >>>> | pre-commit review. Post commit would be especially useful for >>>> us >>>> | too, I think. >>>> | >>>> | Point #2 and #3 are mostly relevant for me, because I mostly >>>> | handle incoming patches. But I think in general it would be >>>> nice, >>>> | and make it a lot easier for newcomers to submit patches, and >>>> us >>>> | to look over them. >>>> | >>>> | Here's an example of a Differential code review: >>>> | >>>> | https://phabricator.haskell.org/D4 >>>> | >>>> | This is a demo using my 'wip/ermsb' patch. You'll need to >>>> create >>>> | an account to login, but it shouldn't be much trouble, you can >>>> | login several ways. I'll fix the login requirement soon. Feel >>>> free >>>> | to read the code, comment on it, and play around. It's more of >>>> a >>>> | demonstration, but real code review would be welcome too. :) >>>> | >>>> | If people are interested in doing this, I can add notes to the >>>> | wiki pages for newcomers, and I'll send another email about >>>> Phab >>>> | so people can understand it a little better. But I want to ask >>>> | first. >>>> | >>>> | There is an argument that our team is so small, code review has >>>> | unnecessary burdens. But I think Phab could help a lot with >>>> | tracking outside patches and getting good reviews for incoming >>>> | patches, and it'll make it easier for newcomers. And >>>> experienced >>>> | pros can probably learn a thing as well. >>>> | >>>> | Again, to be clear, I don't propose we migrate anything to >>>> | Phabricator from, say, Trac. There's no real pressure to do so >>>> and >>>> | it would be tons of work. I only propose we use it for code >>>> | review, which is perfectly fine, and how other projects like >>>> LLVM >>>> | do code review (they use Bugzilla). >>>> | >>>> | I also don't think the usage of Phabricator should be mandatory >>>> | (unless we decide that later because we like it), but I would >>>> like >>>> | to see people use it if possible. >>>> | >>>> | [1] http://phabricator.org >>>> | >>>> | -- >>>> | Regards, >>>> | >>>> | Austin Seipp, Haskell Consultant >>>> | Well-Typed LLP, http://www.well-typed.com/ >>>> | _______________________________________________ >>>> | ghc-devs mailing list >>>> | ghc-devs@haskell.org >>>> | http://www.haskell.org/mailman/listinfo/ghc-devs >>>> >>>> _______________________________________________ >>>> ghc-devs mailing list >>>> ghc-devs@haskell.org >>>> http://www.haskell.org/mailman/listinfo/ghc-devs >>> >>> _______________________________________________ >>> ghc-devs mailing list >>> ghc-devs@haskell.org >>> http://www.haskell.org/mailman/listinfo/ghc-devs >> >> _______________________________________________ >> ghc-devs mailing list >> ghc-devs@haskell.org >> http://www.haskell.org/mailman/listinfo/ghc-devs > > _______________________________________________ > ghc-devs mailing list > ghc-devs@haskell.org > http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards,
Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/ _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

If you have several commits, then use separate 'arc diff' commands to send them to Phab. You can do this even if they depend on each other ("stacked diffs"). My usual workflow is something like this: git checkout -b hacking master .. hack hack .. git commit arc diff HEAD^ .. hack hack .. git commit arc diff HEAD^ Now you have a branch with two separate commits, and two Phab diffs. For landing, you can wait until they're all accepted and then do a single 'git push', or you can push them individually by cherry-picking onto master, pushing, and then rebasing the original branch. The squashing behavior of Phab is intended to squash the *revisions* of a diff into a single commit. Personally I don't use it this way, I just amend the original commit, because it's easier to do stacked diffs this way. Cheers, Simon On 24/06/2014 15:27, Johan Tibell wrote:
I find the automatic squashing to rather harmful to the commit history. So if you have several nice commits that you want to send for review, don't use arc land to commit them, as it will ruin the history. Instead git push them as per normal and use `arc close` (IIRC) to close the code review. I also find it useful to remove lots of the arc gunk that was added to the commit message, to not pollute our revision history with data from some specific tool.
On Tue, Jun 24, 2014 at 4:09 PM, Richard Eisenberg
mailto:eir@cis.upenn.edu> wrote: Thanks so much for writing this! I have some questions:
1) I'm just setting things up on my machine. It says to `arc install-certificate` in my GHC directory. Is it important precisely which clone of GHC my directory is set up against? For example, my "pull" origin is git://git.haskell.org/ghc.git http://git.haskell.org/ghc.git and my "push" origin is ssh://git@git.haskell.org/ghc.git http://git@git.haskell.org/ghc.git. Is this the right set-up? If this bit matters, could you add it to the page? Or, if not, could you comment on what `arc` is pulling from the ghc directory?
2) I'm confused about what, precisely, `arc diff` does. You describe that it updates the review available online. Does that reference some git commits? Do I need to push by `wip` branch before `arc diff`ing? Do I *never* need to push my branch, because `arc diff` pushes it for me? Do I *never* need to push my branch, because Phab uses other ways of moving the code around? For better or worse, I tend to keep my branches local until I'm ready to merge with master, and I want to know if this needs to change.
3) You say "A change cannot be merged until at least one reviewer has signed off." Does this mean "merged with ghc-7.8" (or whatever the current stable branch is)? Or does it mean "merged with master"? The former is the status quo, but with a new route, so to speak. The latter is something new, as several of us push directly to `master` without a review. I'm not against such a change, per se, just trying to understand it.
4) Is it now compulsory to use Phab when contributing? This page makes it sound that way. Again, no complaints -- just trying to understand it.
5) The page says to add `austin` as a reviewer. I would expect `thoughtpolice`. What's up with Phab usernames? Do other people I know and love have Phab usernames distinct from Trac usernames?
6) Who is the reviewer `#ghc`? Is this related to the IRC channel? What does the # signify?
7) I'm baffled by "Landing reviews". The `arc land` bit is clearer, but I'm still unsure of what my local state and the git upstream state must be beforehand for this to work. Will this ask me for a commit message? Will it use the one I specified to Phab during `arc diff`? In general, I'm confused about how much info `arc` pulls from various places to do its work. I know I could learn by doing, but I'm afraid of mashing on the Phab and/or git history as I do so.
8) Some of the same questions surround `arc patch`: What does my git state need to be for this to work?
9) How do I contribute to others' revisions? Or, will this be obvious what it comes to pass?
I realize I've asked a lot here, and it might be too much to expect all of these answers to be on the page. If that's the case, could you perhaps link to relevant manuals or places to learn more? The way `arc` works, in particular, seems like magic; magic is very powerful, but it can be equally dangerous, and so I'd like to learn more.
Thanks so much for doing all this! Richard
On Jun 23, 2014, at 10:44 AM, Austin Seipp
mailto:austin@well-typed.com> wrote: > Hi all, > > I went ahead and took some time to write some stuff down about > Phabricator, including some basic tips on the workflows and > applications here: > > https://ghc.haskell.org/trac/ghc/wiki/Phabricator > > It's definitely going to need more expanding. Do let me know if > anything is confusing. > > On Wed, Jun 18, 2014 at 2:38 AM, Jan Stolarek
mailto:jan.stolarek@p.lodz.pl> wrote: >> Duh, ignore what I wrote. I just realized I'm working on a non-rebased branch :-) >> >> Janek >> >> Dnia środa, 18 czerwca 2014, Jan Stolarek napisał: >>> I read the friendly Arcanist manual and I wonder if we intend to have a >>> default .arcconfig file in the GHC repo? From the docs it seems like a good >>> idea. >>> >>> Janek >>> >>> Dnia wtorek, 17 czerwca 2014, Simon Marlow napisał: >>>> On 13/06/14 10:47, Jan Stolarek wrote: >>>>> It seems that most people are in favour of using Phabricator for code >>>>> review. So what are the next steps? Can we just start using the >>>>> existing phabricator instance? I'm working on some code right now that >>>>> definitely needs reviewing. >>>> >>>> You can use it, and a few of us have already been doing so. There isn't >>>> any Trac integration yet, but it works nicely for patch review. >>>> >>>> There's a short intro doc here: >>>> https://secure.phabricator.com/book/phabricator/article/differential/, >>>> but it's not hard to figure out the basics, and you'll learn by watching >>>> how other people use it. If you go to the Herald tool you have yourself >>>> automatically subscribed to diffs that touch areas of the code that >>>> you're interested in. >>>> >>>> Pro tip: the keyboard shortcuts are really useful, especially "z". Hit >>>> "?" to see all the shortcuts. >>>> >>>> Cheers, >>>> Simon >>>> >>>>> Janek >>>>> >>>>> Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał: >>>>>> On 07/06/2014 07:21, Manuel M T Chakravarty wrote: >>>>>>> So, why not put everything on GutHub and use pull requests and so on? >>>>>> >>>>>> github just isn't great for doing code reviews. No side-by-side diffs, >>>>>> and it sends you a separate email for every single comment, there's no >>>>>> concept of a "review" consisting of multiple inline comments (unless >>>>>> I've missed something). I'm afraid if we were using this for regular >>>>>> reviews I would have to disable the email notifications, which makes >>>>>> it significantly less useful. >>>>>> >>>>>>> SimonM writes that Phabricator is better than GitHub. I’m happy to >>>>>>> believe that, but he also writes that using it requires installing >>>>>>> local software and quite a bit of work. Moreover, I like to add that >>>>>>> lots of people already know how to use GitHub and probably few know >>>>>>> Phabricator. >>>>>>> >>>>>>> So, we are talking about having a somewhat better tool in return for >>>>>>> three very significant disadvantages: (1) local installation, (2) >>>>>>> work to set up and maintain Phabricator, and (3) effort by many >>>>>>> people to learn to use it. >>>>>> >>>>>> Well, you've tipped the balance with "somewhat" and "significant" >>>>>> here, I'd say Phabricator is "significantly" better than github for >>>>>> code reviews, while installing arc is "somewhat" annoying :-) >>>>>> >>>>>> I have to admit it's not a no-brainer, but I do worry that github just >>>>>> wouldn't cut it for doing a lot of code reviewing, whereas I spend my >>>>>> life inside Phabricator so I know it works really well. >>>>>> >>>>>> What's more, github doesn't let you put animated gifs in code reviews. >>>>>> Need I say more? >>>>>> >>>>>> Cheers, >>>>>> Simon >>>>>> >>>>>>> We also have a constant lack of sufficient men power. So, why spend >>>>>>> effort on building our own infrastructure, which will only increase >>>>>>> the hurdle for contributors (as they have to deal with an unknown >>>>>>> system)? Let’s outsource the effort to GitHub. >>>>>>> >>>>>>> Manuel >>>>>>> >>>>>>> Simon Peyton Jones mailto:simonpj@microsoft.com>: >>>>>>>> At the moment GHC's main sources aren't on github, which means that >>>>>>>> that (in my highly imperfect understanding) people can't submit pull >>>>>>>> requests or use their code review mechanisms. Moreover, most people >>>>>>>> don't have commit rights on the main GHC server, so if someone wants >>>>>>>> to offer a patch they can really only do so in textual form attached >>>>>>>> to Trac. People with commit rights can make a branch, but there's a >>>>>>>> danger that over a decade we'll accumulate zillions of dead branches >>>>>>>> which people forgot to delete. I think on github the branch is in a >>>>>>>> different repo, belonging to the patch author. >>>>>>>> >>>>>>>> So we really don't have a good work flow for creating, reviewing, >>>>>>>> modifying, and finally apply patches. I am no expert on these >>>>>>>> matters. If Phabricator would help with that I'm all for it. But >>>>>>>> perhaps there are other alternatives? Or is Phab the lead thing. >>>>>>>> Will it stay around? >>>>>>>> >>>>>>>> Also before going too far I'd really like someone to document the >>>>>>>> workflow carefully, and make sure it works from Windows equally >>>>>>>> well. >>>>>>>> >>>>>>>> I'm not too stressed out about losing the review trail of a patch. >>>>>>>> Much of it will be commenting on stuff that no longer appears in the >>>>>>>> final patch. Anything that's important should appear in a Note in >>>>>>>> the source code; even the commit messages are invisible until you >>>>>>>> really start digging. >>>>>>>> >>>>>>>> Simon >>>>>>>> >>>>>>>> | -----Original Message----- >>>>>>>> | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org mailto:ghc-devs-bounces@haskell.org] On Behalf Of >>>>>>>> | Austin Seipp >>>>>>>> | Sent: 06 June 2014 05:06 >>>>>>>> | To: ghc-devs@haskell.org mailto:ghc-devs@haskell.org >>>>>>>> | Subject: RFC: Phabricator for patches and code review >>>>>>>> | >>>>>>>> | Hello all, >>>>>>>> | >>>>>>>> | Recently, while doing server maintenance, several of the >>>>>>>> | administrators for Haskell.org set up an instance of >>>>>>>> | Phabricator[1], located at https://phabricator.haskell.org >>>>>>>> | >>>>>>>> | For those who aren't aware, Phabricator (or "Phab") is a suite of >>>>>>>> | tools for software development. Think of it like a polished, >>>>>>>> | semi-private GitHub with a lot of applications and tools for all >>>>>>>> | kinds of needs. We've been using it to do issue tracking for >>>>>>>> | Haskell.org maintenance and like it a lot so far. >>>>>>>> | >>>>>>>> | One very nice aspect of Phabricator though is it has a very nice >>>>>>>> | code review tool, called 'Differential', that is very useful. For >>>>>>>> | people who have used a tool like Review Board, it's similar. >>>>>>>> | Furthermore, it has a very convenient userland tool called >>>>>>>> | 'Arcanist' which makes it easy for newcomers to post a review and >>>>>>>> | get it merged when it's ready all from the command line. >>>>>>>> | >>>>>>>> | I'd like to see if people are interested in using Phab _strictly_ >>>>>>>> | for code review of GHC patches. It is a dedicated tool >>>>>>>> | specifically for this, and I think it works much better than Trac >>>>>>>> | or inline GitHub comments. >>>>>>>> | >>>>>>>> | Also, Phab can also support post-commit reviews. So if I touch >>>>>>>> | something in the runtime system and just push, perhaps Simon or >>>>>>>> | Edward would like to look, and they can be alerted right when I do >>>>>>>> | this, and then yell if I did something stupid. >>>>>>>> | >>>>>>>> | Before I go much further, I'd like to ask: is there *any* interest >>>>>>>> | in this? Or are people satisifed with Trac? The primary >>>>>>>> | motivations are roughly, in no particular order: >>>>>>>> | >>>>>>>> | 1) Code review is good for everyone, a good way for people to >>>>>>>> | learn the code and ask questions, and useful to give feedback to >>>>>>>> | newcomers. And even experienced GHC hackers can learn things from >>>>>>>> | reading code, as we all do regularly, or find things that need >>>>>>>> | cleanup. >>>>>>>> | >>>>>>>> | 2) Phabricator in particular makes it very easy to submit patches >>>>>>>> | for review. To submit a patch, I just run the command 'arc diff' >>>>>>>> | and it Does The Right Thing. It also makes it easy to ensure >>>>>>>> | people are *alerted* when a patch might be relevant to them. >>>>>>>> | >>>>>>>> | 3) They can be uploaded and created from the command line, and >>>>>>>> | merged easily afterwords the same way. This is particularly useful >>>>>>>> | for newcomers, and for me. :) >>>>>>>> | >>>>>>>> | 4) Differential is dedicated to code review, and much better at >>>>>>>> | it than just reading patches on Trac IMO. >>>>>>>> | >>>>>>>> | 5) It supports both post-commit code review, as well as >>>>>>>> | pre-commit review. Post commit would be especially useful for us >>>>>>>> | too, I think. >>>>>>>> | >>>>>>>> | Point #2 and #3 are mostly relevant for me, because I mostly >>>>>>>> | handle incoming patches. But I think in general it would be nice, >>>>>>>> | and make it a lot easier for newcomers to submit patches, and us >>>>>>>> | to look over them. >>>>>>>> | >>>>>>>> | Here's an example of a Differential code review: >>>>>>>> | >>>>>>>> | https://phabricator.haskell.org/D4 >>>>>>>> | >>>>>>>> | This is a demo using my 'wip/ermsb' patch. You'll need to create >>>>>>>> | an account to login, but it shouldn't be much trouble, you can >>>>>>>> | login several ways. I'll fix the login requirement soon. Feel free >>>>>>>> | to read the code, comment on it, and play around. It's more of a >>>>>>>> | demonstration, but real code review would be welcome too. :) >>>>>>>> | >>>>>>>> | If people are interested in doing this, I can add notes to the >>>>>>>> | wiki pages for newcomers, and I'll send another email about Phab >>>>>>>> | so people can understand it a little better. But I want to ask >>>>>>>> | first. >>>>>>>> | >>>>>>>> | There is an argument that our team is so small, code review has >>>>>>>> | unnecessary burdens. But I think Phab could help a lot with >>>>>>>> | tracking outside patches and getting good reviews for incoming >>>>>>>> | patches, and it'll make it easier for newcomers. And experienced >>>>>>>> | pros can probably learn a thing as well. >>>>>>>> | >>>>>>>> | Again, to be clear, I don't propose we migrate anything to >>>>>>>> | Phabricator from, say, Trac. There's no real pressure to do so and >>>>>>>> | it would be tons of work. I only propose we use it for code >>>>>>>> | review, which is perfectly fine, and how other projects like LLVM >>>>>>>> | do code review (they use Bugzilla). >>>>>>>> | >>>>>>>> | I also don't think the usage of Phabricator should be mandatory >>>>>>>> | (unless we decide that later because we like it), but I would like >>>>>>>> | to see people use it if possible. >>>>>>>> | >>>>>>>> | [1] http://phabricator.org >>>>>>>> | >>>>>>>> | -- >>>>>>>> | Regards, >>>>>>>> | >>>>>>>> | Austin Seipp, Haskell Consultant >>>>>>>> | Well-Typed LLP, http://www.well-typed.com/ >>>>>>>> | _______________________________________________ >>>>>>>> | ghc-devs mailing list >>>>>>>> | ghc-devs@haskell.org mailto:ghc-devs@haskell.org >>>>>>>> | http://www.haskell.org/mailman/listinfo/ghc-devs >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> ghc-devs mailing list >>>>>>>> ghc-devs@haskell.org mailto:ghc-devs@haskell.org >>>>>>>> http://www.haskell.org/mailman/listinfo/ghc-devs >>>>>>> >>>>>>> _______________________________________________ >>>>>>> ghc-devs mailing list >>>>>>> ghc-devs@haskell.org mailto:ghc-devs@haskell.org >>>>>>> http://www.haskell.org/mailman/listinfo/ghc-devs >>>>>> >>>>>> _______________________________________________ >>>>>> ghc-devs mailing list >>>>>> ghc-devs@haskell.org mailto:ghc-devs@haskell.org >>>>>> http://www.haskell.org/mailman/listinfo/ghc-devs >>>>> >>>>> _______________________________________________ >>>>> ghc-devs mailing list >>>>> ghc-devs@haskell.org mailto:ghc-devs@haskell.org >>>>> http://www.haskell.org/mailman/listinfo/ghc-devs >>> >>> _______________________________________________ >>> ghc-devs mailing list >>> ghc-devs@haskell.org mailto:ghc-devs@haskell.org >>> http://www.haskell.org/mailman/listinfo/ghc-devs >> >> > > > > -- > Regards, > > Austin Seipp, Haskell Consultant > Well-Typed LLP, http://www.well-typed.com/ > _______________________________________________ > ghc-devs mailing list > ghc-devs@haskell.org mailto:ghc-devs@haskell.org > http://www.haskell.org/mailman/listinfo/ghc-devs > _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org mailto:ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

Richard, Thanks, these are all actually really excellent questions.
1) I'm just setting things up on my machine. It says to `arc install-certificate` in my GHC directory. Is it important precisely which clone of GHC my directory is set up against? For example, my "pull" origin is git://git.haskell.org/ghc.git and my "push" origin is ssh://git@git.haskell.org/ghc.git. Is this the right set-up? If this bit matters, could you add it to the page? Or, if not, could you comment on what `arc` is pulling from the ghc directory?
The way 'arc install-certificate' knows what URL to use is based on a file in the GHC repository, called .arcconfig - this tells arcanist where the Phabricator instance is, and what the project callsign is. GHC's copy is here: https://github.com/ghc/ghc/blob/master/.arcconfig So all you need to do is run 'install-certificate' inside the main GHC repo, and you're done! If you've got a copy of HEAD from the past two weeks or so, you should be golden. Once install-certificate is done, it will write the certificate into a file called ~/.arcrc, so you don't have to install it again.
2) I'm confused about what, precisely, `arc diff` does. You describe that it updates the review available online. Does that reference some git commits? Do I need to push by `wip` branch before `arc diff`ing? Do I *never* need to push my branch, because `arc diff` pushes it for me? Do I *never* need to push my branch, because Phab uses other ways of moving the code around? For better or worse, I tend to keep my branches local until I'm ready to merge with master, and I want to know if this needs to change.
You _never_ have to push a branch upstream if you don't want to (but if you'd like to keep your changes in a safe place until you merge them, feel free to put them on a wip/* branch!) What 'arc diff' does is this: when you run it, it calculates which commits you have made. It does this by comparing your current git repository to that of the *upstream* repository, the GHC repo. When it calculates the set of commits, 'arc diff' then sends them all for review. When you make a new commit, and say 'arc diff' again, it updates it.
3) You say "A change cannot be merged until at least one reviewer has signed off." Does this mean "merged with ghc-7.8" (or whatever the current stable branch is)? Or does it mean "merged with master"? The former is the status quo, but with a new route, so to speak. The latter is something new, as several of us push directly to `master` without a review. I'm not against such a change, per se, just trying to understand it.
Not quite. All changes go into master first, after it has been reviewed. Afterwords, they may be picked onto the stable branch by me or Herbert. My only statement is that we cannot merge a patch from Phab to master until at least one person has given it the OK. If you put a review on Phabricator, I believe you should at least wait for one person to review it. That is, after all, what Phabricator is for! :) But note: you are *never* required to use Phabricator for your changes! If your workflow is good for you, I don't want to interrupt it. Instead, I want you to submit reviews so other people can read and understand your work - existing committers *are not* forced to do this. For example Richard, you obviously keep in close touch with Simon, so he's likely aware of your changes and their needs. There's no reason for you to ask SPJ for a review on a change you already have a handle on - just go ahead and push it!
4) Is it now compulsory to use Phab when contributing? This page makes it sound that way. Again, no complaints -- just trying to understand it.
Nope! It might be so later (for external contributors without commit access), but I first need to document all the workflows so contributors can get a handle on it. So thanks for being the guinea pig :) But it won't ever be compulsory for all committers. Again, I want people to use Phab so they can have others read, understand, and comment about their work. There's no hard-line rule on if you should or should not submit a review - I will totally trust your judgement as to what you think is your best workflow.
5) The page says to add `austin` as a reviewer. I would expect `thoughtpolice`. What's up with Phab usernames? Do other people I know and love have Phab usernames distinct from Trac usernames?
Sorry - I'm the only outlier. I believe every other person has the same username except for me. :P (This is partly due to the fact we use Phabricator for other Haskell.org projects as well, and we didn't explicitly plan to use it for GHC initially - so 'austin' was the name I picked when first setting up the instance).
6) Who is the reviewer `#ghc`? Is this related to the IRC channel? What does the # signify?
Good question I totally forgot about. In Phabricator, there is a notion of 'projects', which have members. There is a project known as 'GHC', and in Phab, you can refer to this group of people within the project by a hashtag, essentially. So `#ghc` means "All people who are in the GHC project". The # is just a way of referring to a project or group, basically. Here's the current listing of people in the GHC project: https://phabricator.haskell.org/project/view/2/ - you should be able to (and feel free to) add yourself to that group from that linked page.
7) I'm baffled by "Landing reviews". The `arc land` bit is clearer, but I'm still unsure of what my local state and the git upstream state must be beforehand for this to work. Will this ask me for a commit message? Will it use the one I specified to Phab during `arc diff`? In general, I'm confused about how much info `arc` pulls from various places to do its work. I know I could learn by doing, but I'm afraid of mashing on the Phab and/or git history as I do so.
This is a good question, but it's one probably best answered with an example. I'll work something into the page to make this easier to understand.
8) Some of the same questions surround `arc patch`: What does my git state need to be for this to work?
Ditto with this one - I'll work it into a tangible example to make it clearer.
9) How do I contribute to others' revisions? Or, will this be obvious
what it comes to pass?
Normally, you just do things 'the git way', so don't let Phab get in
your way here. If you want to contribute to someone elses patches -
work with them and send them your patch! Then they can update the
review with you and their own work included.
However, there is *another* related notion in Phab which is that of
"commandeering" a review. Which should be pretty self explanatory: you
take it over, and now *you* are in control of submitting new diffs.
Sometimes this may happen if I work on something, leave it rotting,
and someone else may want to pick it up.
Anyway, thanks for all the excellent questions Richard, these were
things that definitely would have confused others, so I'm glad you
asked. I'll update the wiki page to be clearer.
On Tue, Jun 24, 2014 at 9:09 AM, Richard Eisenberg
Thanks so much for writing this! I have some questions:
1) I'm just setting things up on my machine. It says to `arc install-certificate` in my GHC directory. Is it important precisely which clone of GHC my directory is set up against? For example, my "pull" origin is git://git.haskell.org/ghc.git and my "push" origin is ssh://git@git.haskell.org/ghc.git. Is this the right set-up? If this bit matters, could you add it to the page? Or, if not, could you comment on what `arc` is pulling from the ghc directory?
2) I'm confused about what, precisely, `arc diff` does. You describe that it updates the review available online. Does that reference some git commits? Do I need to push by `wip` branch before `arc diff`ing? Do I *never* need to push my branch, because `arc diff` pushes it for me? Do I *never* need to push my branch, because Phab uses other ways of moving the code around? For better or worse, I tend to keep my branches local until I'm ready to merge with master, and I want to know if this needs to change.
3) You say "A change cannot be merged until at least one reviewer has signed off." Does this mean "merged with ghc-7.8" (or whatever the current stable branch is)? Or does it mean "merged with master"? The former is the status quo, but with a new route, so to speak. The latter is something new, as several of us push directly to `master` without a review. I'm not against such a change, per se, just trying to understand it.
4) Is it now compulsory to use Phab when contributing? This page makes it sound that way. Again, no complaints -- just trying to understand it.
5) The page says to add `austin` as a reviewer. I would expect `thoughtpolice`. What's up with Phab usernames? Do other people I know and love have Phab usernames distinct from Trac usernames?
6) Who is the reviewer `#ghc`? Is this related to the IRC channel? What does the # signify?
7) I'm baffled by "Landing reviews". The `arc land` bit is clearer, but I'm still unsure of what my local state and the git upstream state must be beforehand for this to work. Will this ask me for a commit message? Will it use the one I specified to Phab during `arc diff`? In general, I'm confused about how much info `arc` pulls from various places to do its work. I know I could learn by doing, but I'm afraid of mashing on the Phab and/or git history as I do so.
8) Some of the same questions surround `arc patch`: What does my git state need to be for this to work?
9) How do I contribute to others' revisions? Or, will this be obvious what it comes to pass?
I realize I've asked a lot here, and it might be too much to expect all of these answers to be on the page. If that's the case, could you perhaps link to relevant manuals or places to learn more? The way `arc` works, in particular, seems like magic; magic is very powerful, but it can be equally dangerous, and so I'd like to learn more.
Thanks so much for doing all this! Richard
On Jun 23, 2014, at 10:44 AM, Austin Seipp
wrote: Hi all,
I went ahead and took some time to write some stuff down about Phabricator, including some basic tips on the workflows and applications here:
https://ghc.haskell.org/trac/ghc/wiki/Phabricator
It's definitely going to need more expanding. Do let me know if anything is confusing.
On Wed, Jun 18, 2014 at 2:38 AM, Jan Stolarek
wrote: Duh, ignore what I wrote. I just realized I'm working on a non-rebased branch :-)
Janek
Dnia środa, 18 czerwca 2014, Jan Stolarek napisał:
I read the friendly Arcanist manual and I wonder if we intend to have a default .arcconfig file in the GHC repo? From the docs it seems like a good idea.
Janek
Dnia wtorek, 17 czerwca 2014, Simon Marlow napisał:
On 13/06/14 10:47, Jan Stolarek wrote:
It seems that most people are in favour of using Phabricator for code review. So what are the next steps? Can we just start using the existing phabricator instance? I'm working on some code right now that definitely needs reviewing.
You can use it, and a few of us have already been doing so. There isn't any Trac integration yet, but it works nicely for patch review.
There's a short intro doc here: https://secure.phabricator.com/book/phabricator/article/differential/, but it's not hard to figure out the basics, and you'll learn by watching how other people use it. If you go to the Herald tool you have yourself automatically subscribed to diffs that touch areas of the code that you're interested in.
Pro tip: the keyboard shortcuts are really useful, especially "z". Hit "?" to see all the shortcuts.
Cheers, Simon
Janek
Dnia niedziela, 8 czerwca 2014, Simon Marlow napisał: > On 07/06/2014 07:21, Manuel M T Chakravarty wrote: >> So, why not put everything on GutHub and use pull requests and so on? > > github just isn't great for doing code reviews. No side-by-side diffs, > and it sends you a separate email for every single comment, there's no > concept of a "review" consisting of multiple inline comments (unless > I've missed something). I'm afraid if we were using this for regular > reviews I would have to disable the email notifications, which makes > it significantly less useful. > >> SimonM writes that Phabricator is better than GitHub. I’m happy to >> believe that, but he also writes that using it requires installing >> local software and quite a bit of work. Moreover, I like to add that >> lots of people already know how to use GitHub and probably few know >> Phabricator. >> >> So, we are talking about having a somewhat better tool in return for >> three very significant disadvantages: (1) local installation, (2) >> work to set up and maintain Phabricator, and (3) effort by many >> people to learn to use it. > > Well, you've tipped the balance with "somewhat" and "significant" > here, I'd say Phabricator is "significantly" better than github for > code reviews, while installing arc is "somewhat" annoying :-) > > I have to admit it's not a no-brainer, but I do worry that github just > wouldn't cut it for doing a lot of code reviewing, whereas I spend my > life inside Phabricator so I know it works really well. > > What's more, github doesn't let you put animated gifs in code reviews. > Need I say more? > > Cheers, > Simon > >> We also have a constant lack of sufficient men power. So, why spend >> effort on building our own infrastructure, which will only increase >> the hurdle for contributors (as they have to deal with an unknown >> system)? Let’s outsource the effort to GitHub. >> >> Manuel >> >> Simon Peyton Jones
: >>> At the moment GHC's main sources aren't on github, which means that >>> that (in my highly imperfect understanding) people can't submit pull >>> requests or use their code review mechanisms. Moreover, most people >>> don't have commit rights on the main GHC server, so if someone wants >>> to offer a patch they can really only do so in textual form attached >>> to Trac. People with commit rights can make a branch, but there's a >>> danger that over a decade we'll accumulate zillions of dead branches >>> which people forgot to delete. I think on github the branch is in a >>> different repo, belonging to the patch author. >>> >>> So we really don't have a good work flow for creating, reviewing, >>> modifying, and finally apply patches. I am no expert on these >>> matters. If Phabricator would help with that I'm all for it. But >>> perhaps there are other alternatives? Or is Phab the lead thing. >>> Will it stay around? >>> >>> Also before going too far I'd really like someone to document the >>> workflow carefully, and make sure it works from Windows equally >>> well. >>> >>> I'm not too stressed out about losing the review trail of a patch. >>> Much of it will be commenting on stuff that no longer appears in the >>> final patch. Anything that's important should appear in a Note in >>> the source code; even the commit messages are invisible until you >>> really start digging. >>> >>> Simon >>> >>> | -----Original Message----- >>> | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of >>> | Austin Seipp >>> | Sent: 06 June 2014 05:06 >>> | To: ghc-devs@haskell.org >>> | Subject: RFC: Phabricator for patches and code review >>> | >>> | Hello all, >>> | >>> | Recently, while doing server maintenance, several of the >>> | administrators for Haskell.org set up an instance of >>> | Phabricator[1], located at https://phabricator.haskell.org >>> | >>> | For those who aren't aware, Phabricator (or "Phab") is a suite of >>> | tools for software development. Think of it like a polished, >>> | semi-private GitHub with a lot of applications and tools for all >>> | kinds of needs. We've been using it to do issue tracking for >>> | Haskell.org maintenance and like it a lot so far. >>> | >>> | One very nice aspect of Phabricator though is it has a very nice >>> | code review tool, called 'Differential', that is very useful. For >>> | people who have used a tool like Review Board, it's similar. >>> | Furthermore, it has a very convenient userland tool called >>> | 'Arcanist' which makes it easy for newcomers to post a review and >>> | get it merged when it's ready all from the command line. >>> | >>> | I'd like to see if people are interested in using Phab _strictly_ >>> | for code review of GHC patches. It is a dedicated tool >>> | specifically for this, and I think it works much better than Trac >>> | or inline GitHub comments. >>> | >>> | Also, Phab can also support post-commit reviews. So if I touch >>> | something in the runtime system and just push, perhaps Simon or >>> | Edward would like to look, and they can be alerted right when I do >>> | this, and then yell if I did something stupid. >>> | >>> | Before I go much further, I'd like to ask: is there *any* interest >>> | in this? Or are people satisifed with Trac? The primary >>> | motivations are roughly, in no particular order: >>> | >>> | 1) Code review is good for everyone, a good way for people to >>> | learn the code and ask questions, and useful to give feedback to >>> | newcomers. And even experienced GHC hackers can learn things from >>> | reading code, as we all do regularly, or find things that need >>> | cleanup. >>> | >>> | 2) Phabricator in particular makes it very easy to submit patches >>> | for review. To submit a patch, I just run the command 'arc diff' >>> | and it Does The Right Thing. It also makes it easy to ensure >>> | people are *alerted* when a patch might be relevant to them. >>> | >>> | 3) They can be uploaded and created from the command line, and >>> | merged easily afterwords the same way. This is particularly useful >>> | for newcomers, and for me. :) >>> | >>> | 4) Differential is dedicated to code review, and much better at >>> | it than just reading patches on Trac IMO. >>> | >>> | 5) It supports both post-commit code review, as well as >>> | pre-commit review. Post commit would be especially useful for us >>> | too, I think. >>> | >>> | Point #2 and #3 are mostly relevant for me, because I mostly >>> | handle incoming patches. But I think in general it would be nice, >>> | and make it a lot easier for newcomers to submit patches, and us >>> | to look over them. >>> | >>> | Here's an example of a Differential code review: >>> | >>> | https://phabricator.haskell.org/D4 >>> | >>> | This is a demo using my 'wip/ermsb' patch. You'll need to create >>> | an account to login, but it shouldn't be much trouble, you can >>> | login several ways. I'll fix the login requirement soon. Feel free >>> | to read the code, comment on it, and play around. It's more of a >>> | demonstration, but real code review would be welcome too. :) >>> | >>> | If people are interested in doing this, I can add notes to the >>> | wiki pages for newcomers, and I'll send another email about Phab >>> | so people can understand it a little better. But I want to ask >>> | first. >>> | >>> | There is an argument that our team is so small, code review has >>> | unnecessary burdens. But I think Phab could help a lot with >>> | tracking outside patches and getting good reviews for incoming >>> | patches, and it'll make it easier for newcomers. And experienced >>> | pros can probably learn a thing as well. >>> | >>> | Again, to be clear, I don't propose we migrate anything to >>> | Phabricator from, say, Trac. There's no real pressure to do so and >>> | it would be tons of work. I only propose we use it for code >>> | review, which is perfectly fine, and how other projects like LLVM >>> | do code review (they use Bugzilla). >>> | >>> | I also don't think the usage of Phabricator should be mandatory >>> | (unless we decide that later because we like it), but I would like >>> | to see people use it if possible. >>> | >>> | [1] http://phabricator.org >>> | >>> | -- >>> | Regards, >>> | >>> | Austin Seipp, Haskell Consultant >>> | Well-Typed LLP, http://www.well-typed.com/ >>> | _______________________________________________ >>> | ghc-devs mailing list >>> | ghc-devs@haskell.org >>> | http://www.haskell.org/mailman/listinfo/ghc-devs >>> >>> _______________________________________________ >>> ghc-devs mailing list >>> ghc-devs@haskell.org >>> http://www.haskell.org/mailman/listinfo/ghc-devs >> >> _______________________________________________ >> ghc-devs mailing list >> ghc-devs@haskell.org >> http://www.haskell.org/mailman/listinfo/ghc-devs > > _______________________________________________ > ghc-devs mailing list > ghc-devs@haskell.org > http://www.haskell.org/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards,
Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/ _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

Austin, replying by email is good, and your responses are helpful. But modifying the wiki page so that the next Richard will not even have to ask the questions is 10x better! Maybe you can just cut/paste text into it....
Simon
| -----Original Message-----
| From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin
| Seipp
| Sent: 24 June 2014 18:13
| To: Richard Eisenberg
| Cc: ghc-devs@haskell.org
| Subject: Re: Phabricator for patches and code review
|
| Richard,
|
| Thanks, these are all actually really excellent questions.
|
| > 1) I'm just setting things up on my machine. It says to `arc install-
| certificate` in my GHC directory. Is it important precisely which clone
| of GHC my directory is set up against? For example, my "pull" origin is
| git://git.haskell.org/ghc.git and my "push" origin is
| ssh://git@git.haskell.org/ghc.git. Is this the right set-up? If this bit
| matters, could you add it to the page? Or, if not, could you comment on
| what `arc` is pulling from the ghc directory?
|
| The way 'arc install-certificate' knows what URL to use is based on a
| file in the GHC repository, called .arcconfig - this tells arcanist
| where the Phabricator instance is, and what the project callsign is.
| GHC's copy is here: https://github.com/ghc/ghc/blob/master/.arcconfig
|
| So all you need to do is run 'install-certificate' inside the main GHC
| repo, and you're done! If you've got a copy of HEAD from the past two
| weeks or so, you should be golden. Once install-certificate is done,
| it will write the certificate into a file called ~/.arcrc, so you
| don't have to install it again.
|
| > 2) I'm confused about what, precisely, `arc diff` does. You describe
| that it updates the review available online. Does that reference some git
| commits? Do I need to push by `wip` branch before `arc diff`ing? Do I
| *never* need to push my branch, because `arc diff` pushes it for me? Do I
| *never* need to push my branch, because Phab uses other ways of moving
| the code around? For better or worse, I tend to keep my branches local
| until I'm ready to merge with master, and I want to know if this needs to
| change.
|
| You _never_ have to push a branch upstream if you don't want to (but
| if you'd like to keep your changes in a safe place until you merge
| them, feel free to put them on a wip/* branch!)
|
| What 'arc diff' does is this: when you run it, it calculates which
| commits you have made. It does this by comparing your current git
| repository to that of the *upstream* repository, the GHC repo. When it
| calculates the set of commits, 'arc diff' then sends them all for
| review. When you make a new commit, and say 'arc diff' again, it
| updates it.
|
| > 3) You say "A change cannot be merged until at least one reviewer has
| signed off." Does this mean "merged with ghc-7.8" (or whatever the
| current stable branch is)? Or does it mean "merged with master"? The
| former is the status quo, but with a new route, so to speak. The latter
| is something new, as several of us push directly to `master` without a
| review. I'm not against such a change, per se, just trying to understand
| it.
|
| Not quite. All changes go into master first, after it has been
| reviewed. Afterwords, they may be picked onto the stable branch by me
| or Herbert.
|
| My only statement is that we cannot merge a patch from Phab to master
| until at least one person has given it the OK. If you put a review on
| Phabricator, I believe you should at least wait for one person to
| review it. That is, after all, what Phabricator is for! :)
|
| But note: you are *never* required to use Phabricator for your
| changes! If your workflow is good for you, I don't want to interrupt
| it. Instead, I want you to submit reviews so other people can read and
| understand your work - existing committers *are not* forced to do
| this. For example Richard, you obviously keep in close touch with
| Simon, so he's likely aware of your changes and their needs. There's
| no reason for you to ask SPJ for a review on a change you already have
| a handle on - just go ahead and push it!
|
| > 4) Is it now compulsory to use Phab when contributing? This page makes
| it sound that way. Again, no complaints -- just trying to understand it.
|
| Nope! It might be so later (for external contributors without commit
| access), but I first need to document all the workflows so
| contributors can get a handle on it. So thanks for being the guinea
| pig :) But it won't ever be compulsory for all committers.
|
| Again, I want people to use Phab so they can have others read,
| understand, and comment about their work. There's no hard-line rule on
| if you should or should not submit a review - I will totally trust
| your judgement as to what you think is your best workflow.
|
| > 5) The page says to add `austin` as a reviewer. I would expect
| `thoughtpolice`. What's up with Phab usernames? Do other people I know
| and love have Phab usernames distinct from Trac usernames?
|
| Sorry - I'm the only outlier. I believe every other person has the
| same username except for me. :P (This is partly due to the fact we use
| Phabricator for other Haskell.org projects as well, and we didn't
| explicitly plan to use it for GHC initially - so 'austin' was the name
| I picked when first setting up the instance).
|
| > 6) Who is the reviewer `#ghc`? Is this related to the IRC channel? What
| does the # signify?
|
| Good question I totally forgot about. In Phabricator, there is a
| notion of 'projects', which have members. There is a project known as
| 'GHC', and in Phab, you can refer to this group of people within the
| project by a hashtag, essentially. So `#ghc` means "All people who are
| in the GHC project". The # is just a way of referring to a project or
| group, basically.
|
| Here's the current listing of people in the GHC project:
| https://phabricator.haskell.org/project/view/2/ - you should be able
| to (and feel free to) add yourself to that group from that linked
| page.
|
| > 7) I'm baffled by "Landing reviews". The `arc land` bit is clearer, but
| I'm still unsure of what my local state and the git upstream state must
| be beforehand for this to work. Will this ask me for a commit message?
| Will it use the one I specified to Phab during `arc diff`? In general,
| I'm confused about how much info `arc` pulls from various places to do
| its work. I know I could learn by doing, but I'm afraid of mashing on the
| Phab and/or git history as I do so.
|
| This is a good question, but it's one probably best answered with an
| example. I'll work something into the page to make this easier to
| understand.
|
| > 8) Some of the same questions surround `arc patch`: What does my git
| state need to be for this to work?
|
| Ditto with this one - I'll work it into a tangible example to make it
| clearer.
|
| 9) How do I contribute to others' revisions? Or, will this be obvious
| what it comes to pass?
|
| Normally, you just do things 'the git way', so don't let Phab get in
| your way here. If you want to contribute to someone elses patches -
| work with them and send them your patch! Then they can update the
| review with you and their own work included.
|
| However, there is *another* related notion in Phab which is that of
| "commandeering" a review. Which should be pretty self explanatory: you
| take it over, and now *you* are in control of submitting new diffs.
| Sometimes this may happen if I work on something, leave it rotting,
| and someone else may want to pick it up.
|
| Anyway, thanks for all the excellent questions Richard, these were
| things that definitely would have confused others, so I'm glad you
| asked. I'll update the wiki page to be clearer.
|
| On Tue, Jun 24, 2014 at 9:09 AM, Richard Eisenberg

PS I couldn't get past the login box at https://phabricator.haskell.org/D4 | -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs

I'm fiddling with the access policies a bit, to make it all publicly
viewable. I thought I fixed it, but apparently not...
In the mean time, you can just register an account (with a
username/password, or just use your existing GitHub login!) and
everything will be viewable.
On Fri, Jun 6, 2014 at 1:29 AM, Simon Peyton Jones
PS I couldn't get past the login box at https://phabricator.haskell.org/D4
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Austin | Seipp | Sent: 06 June 2014 05:06 | To: ghc-devs@haskell.org | Subject: RFC: Phabricator for patches and code review | | Hello all, | | Recently, while doing server maintenance, several of the administrators | for Haskell.org set up an instance of Phabricator[1], located at | https://phabricator.haskell.org | | For those who aren't aware, Phabricator (or "Phab") is a suite of tools | for software development. Think of it like a polished, semi-private | GitHub with a lot of applications and tools for all kinds of needs. | We've been using it to do issue tracking for Haskell.org maintenance and | like it a lot so far. | | One very nice aspect of Phabricator though is it has a very nice code | review tool, called 'Differential', that is very useful. For people who | have used a tool like Review Board, it's similar. Furthermore, it has a | very convenient userland tool called 'Arcanist' which makes it easy for | newcomers to post a review and get it merged when it's ready all from | the command line. | | I'd like to see if people are interested in using Phab _strictly_ for | code review of GHC patches. It is a dedicated tool specifically for | this, and I think it works much better than Trac or inline GitHub | comments. | | Also, Phab can also support post-commit reviews. So if I touch something | in the runtime system and just push, perhaps Simon or Edward would like | to look, and they can be alerted right when I do this, and then yell if | I did something stupid. | | Before I go much further, I'd like to ask: is there *any* interest in | this? Or are people satisifed with Trac? The primary motivations are | roughly, in no particular order: | | 1) Code review is good for everyone, a good way for people to learn the | code and ask questions, and useful to give feedback to newcomers. | And even experienced GHC hackers can learn things from reading code, as | we all do regularly, or find things that need cleanup. | | 2) Phabricator in particular makes it very easy to submit patches for | review. To submit a patch, I just run the command 'arc diff' and it Does | The Right Thing. It also makes it easy to ensure people are | *alerted* when a patch might be relevant to them. | | 3) They can be uploaded and created from the command line, and merged | easily afterwords the same way. This is particularly useful for | newcomers, and for me. :) | | 4) Differential is dedicated to code review, and much better at it than | just reading patches on Trac IMO. | | 5) It supports both post-commit code review, as well as pre-commit | review. Post commit would be especially useful for us too, I think. | | Point #2 and #3 are mostly relevant for me, because I mostly handle | incoming patches. But I think in general it would be nice, and make it a | lot easier for newcomers to submit patches, and us to look over them. | | Here's an example of a Differential code review: | | https://phabricator.haskell.org/D4 | | This is a demo using my 'wip/ermsb' patch. You'll need to create an | account to login, but it shouldn't be much trouble, you can login | several ways. I'll fix the login requirement soon. Feel free to read the | code, comment on it, and play around. It's more of a demonstration, but | real code review would be welcome too. :) | | If people are interested in doing this, I can add notes to the wiki | pages for newcomers, and I'll send another email about Phab so people | can understand it a little better. But I want to ask first. | | There is an argument that our team is so small, code review has | unnecessary burdens. But I think Phab could help a lot with tracking | outside patches and getting good reviews for incoming patches, and it'll | make it easier for newcomers. And experienced pros can probably learn a | thing as well. | | Again, to be clear, I don't propose we migrate anything to Phabricator | from, say, Trac. There's no real pressure to do so and it would be tons | of work. I only propose we use it for code review, which is perfectly | fine, and how other projects like LLVM do code review (they use | Bugzilla). | | I also don't think the usage of Phabricator should be mandatory (unless | we decide that later because we like it), but I would like to see people | use it if possible. | | [1] http://phabricator.org | | -- | Regards, | | Austin Seipp, Haskell Consultant | Well-Typed LLP, http://www.well-typed.com/ | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

2014-06-06 7:05 GMT+03:00 Austin Seipp
2) Phabricator in particular makes it very easy to submit patches for review. To submit a patch, I just run the command 'arc diff' and it Does The Right Thing. It also makes it easy to ensure people are *alerted* when a patch might be relevant to them.
This sounds really good. I was thinking about sending an email about this for a while now. I'm reading some parts of GHC and there are lots of small patches I'd like to submit for reviews. Most of the time these are <10 lines of changes. But trac makes everything so hard and the interface is so horrible, I'm ending up not sending the patch. Also, testing is a huge problem for me. I can't test GHC on my laptop(which is my only development environment) because it takes forever to finish. With something like Github and a CI server(Jenkins/Travis/whatever) integrated to the Github repository that runs tests on pull request, it would be super easy for new contributors to submit small patches. As far as I can understand(altough currently I can't see how to send a patch) Phabricator helps sending pull requests/patches, but does it help with testing too? --- Ömer Sinan Ağacan http://osa1.net

No, at the moment Phabricator is not integrated with any testing
functionality. It could be, but that would be a bit of work I think to
integrate with GHC's ./validate process. It would be nice to have
long-term, however, but I don't think it's necessary right now - I run
./validate before every push out anyway just in case.
If you do need a machine to help run builds, a recommendation: sign up
for AWS. A GHC build can be done in < 1hr on those machines, and an
x1.xlarge for example would cost you about .28c USD to run for that
period. It's less than optimal, but I'm not the only one who has
resorted to this on less-than-adequate machines...
Another recommendation is to make sure you've got the right build
settings, which can have a pretty dramatic impact on build times. See
here: https://ghc.haskell.org/trac/ghc/wiki/Building/Hacking and also
https://ghc.haskell.org/trac/ghc/wiki/Building/Using#HowtomakeGHCbuildquickl...
On Fri, Jun 6, 2014 at 3:13 AM, Ömer Sinan Ağacan
2014-06-06 7:05 GMT+03:00 Austin Seipp
: 2) Phabricator in particular makes it very easy to submit patches for review. To submit a patch, I just run the command 'arc diff' and it Does The Right Thing. It also makes it easy to ensure people are *alerted* when a patch might be relevant to them.
This sounds really good. I was thinking about sending an email about this for a while now. I'm reading some parts of GHC and there are lots of small patches I'd like to submit for reviews. Most of the time these are <10 lines of changes. But trac makes everything so hard and the interface is so horrible, I'm ending up not sending the patch. Also, testing is a huge problem for me. I can't test GHC on my laptop(which is my only development environment) because it takes forever to finish.
With something like Github and a CI server(Jenkins/Travis/whatever) integrated to the Github repository that runs tests on pull request, it would be super easy for new contributors to submit small patches.
As far as I can understand(altough currently I can't see how to send a patch) Phabricator helps sending pull requests/patches, but does it help with testing too?
--- Ömer Sinan Ağacan http://osa1.net
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

Piggybacking a bit on Ömer's point:
It is often the case that something flies by that I can fix in a few moments (for example, #9163) but that I have to defer until I have enough time for a GHC hacking session. Making even a tiny patch requires that I'm up-to-date, that my unchanged tree compiles (and, if I have time, validates!), and then, after the patch, that everything validates again. This generally means that I don't touch GHC unless I have 3+ hours *that day* to devote to GHC -- there are enough commits that I don't want things getting too stale if I can't validate the same day I update. Compounding the problem, my desire for efficiency leads me to accumulate GHC tasks, meaning that my time requirements become even worse, making it harder to get anything done.
It sounds like, perhaps, Phab isn't the answer, but a way to create a lightweight, *untested* patch that doesn't require all of this would be simply wonderful. Experience has told me even to be scared of just adding comments without a validate!
Is there such a facility now? How hard would it be to create one?
Richard
On Jun 6, 2014, at 8:59 AM, Austin Seipp
No, at the moment Phabricator is not integrated with any testing functionality. It could be, but that would be a bit of work I think to integrate with GHC's ./validate process. It would be nice to have long-term, however, but I don't think it's necessary right now - I run ./validate before every push out anyway just in case.
If you do need a machine to help run builds, a recommendation: sign up for AWS. A GHC build can be done in < 1hr on those machines, and an x1.xlarge for example would cost you about .28c USD to run for that period. It's less than optimal, but I'm not the only one who has resorted to this on less-than-adequate machines...
Another recommendation is to make sure you've got the right build settings, which can have a pretty dramatic impact on build times. See here: https://ghc.haskell.org/trac/ghc/wiki/Building/Hacking and also https://ghc.haskell.org/trac/ghc/wiki/Building/Using#HowtomakeGHCbuildquickl...
On Fri, Jun 6, 2014 at 3:13 AM, Ömer Sinan Ağacan
wrote: 2014-06-06 7:05 GMT+03:00 Austin Seipp
: 2) Phabricator in particular makes it very easy to submit patches for review. To submit a patch, I just run the command 'arc diff' and it Does The Right Thing. It also makes it easy to ensure people are *alerted* when a patch might be relevant to them.
This sounds really good. I was thinking about sending an email about this for a while now. I'm reading some parts of GHC and there are lots of small patches I'd like to submit for reviews. Most of the time these are <10 lines of changes. But trac makes everything so hard and the interface is so horrible, I'm ending up not sending the patch. Also, testing is a huge problem for me. I can't test GHC on my laptop(which is my only development environment) because it takes forever to finish.
With something like Github and a CI server(Jenkins/Travis/whatever) integrated to the Github repository that runs tests on pull request, it would be super easy for new contributors to submit small patches.
As far as I can understand(altough currently I can't see how to send a patch) Phabricator helps sending pull requests/patches, but does it help with testing too?
--- Ömer Sinan Ağacan http://osa1.net
-- Regards,
Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/ _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs

Richard, I think Phabricator would suit this just fine.
Here's (roughly) how Phabricator will work from a GHC hackers point of view.
1) I have a thing I want. Make a branch, hack hack hack away.
$ git branch new-thing
$ git checkout new-thing
$ emacs ....
$ git commit -a -m "Add minor new thing"
2) Use the CLI tool, Arcanist, to submit a diff:
$ arc diff
This uploads your changes on your branch to the web server. (The CLI
interface will explain in detail what will happen.) Done.
When you do #2, this will take the patch, upload it to Phabricator,
and tie a lot of metadata to it. As an example, you can see D4, which
I posted before: https://phabricator.haskell.org/D4 - Herbert and
Carter for example commented before, and I've updated the review since
then.
After a review is on differential, we can review it, easily merge it
if we think it's acceptable and test it - or just leave it alone until
you're ready for us to review it later.
Most importantly, things uploaded for code review can absolutely be
work-in-progress, untested, and it's *easy* to do upload them. Just
one command. I think that's just what you want. Just make a branch,
and create a review on Differential. You can come back to it later,
and in the mean time we can review it, track it, or even merge it
ourselves if we have time.
On Fri, Jun 6, 2014 at 9:09 AM, Richard Eisenberg
Piggybacking a bit on Ömer's point:
It is often the case that something flies by that I can fix in a few moments (for example, #9163) but that I have to defer until I have enough time for a GHC hacking session. Making even a tiny patch requires that I'm up-to-date, that my unchanged tree compiles (and, if I have time, validates!), and then, after the patch, that everything validates again. This generally means that I don't touch GHC unless I have 3+ hours *that day* to devote to GHC -- there are enough commits that I don't want things getting too stale if I can't validate the same day I update. Compounding the problem, my desire for efficiency leads me to accumulate GHC tasks, meaning that my time requirements become even worse, making it harder to get anything done.
It sounds like, perhaps, Phab isn't the answer, but a way to create a lightweight, *untested* patch that doesn't require all of this would be simply wonderful. Experience has told me even to be scared of just adding comments without a validate!
Is there such a facility now? How hard would it be to create one?
Richard
On Jun 6, 2014, at 8:59 AM, Austin Seipp
wrote: No, at the moment Phabricator is not integrated with any testing functionality. It could be, but that would be a bit of work I think to integrate with GHC's ./validate process. It would be nice to have long-term, however, but I don't think it's necessary right now - I run ./validate before every push out anyway just in case.
If you do need a machine to help run builds, a recommendation: sign up for AWS. A GHC build can be done in < 1hr on those machines, and an x1.xlarge for example would cost you about .28c USD to run for that period. It's less than optimal, but I'm not the only one who has resorted to this on less-than-adequate machines...
Another recommendation is to make sure you've got the right build settings, which can have a pretty dramatic impact on build times. See here: https://ghc.haskell.org/trac/ghc/wiki/Building/Hacking and also https://ghc.haskell.org/trac/ghc/wiki/Building/Using#HowtomakeGHCbuildquickl...
On Fri, Jun 6, 2014 at 3:13 AM, Ömer Sinan Ağacan
wrote: 2014-06-06 7:05 GMT+03:00 Austin Seipp
: 2) Phabricator in particular makes it very easy to submit patches for review. To submit a patch, I just run the command 'arc diff' and it Does The Right Thing. It also makes it easy to ensure people are *alerted* when a patch might be relevant to them.
This sounds really good. I was thinking about sending an email about this for a while now. I'm reading some parts of GHC and there are lots of small patches I'd like to submit for reviews. Most of the time these are <10 lines of changes. But trac makes everything so hard and the interface is so horrible, I'm ending up not sending the patch. Also, testing is a huge problem for me. I can't test GHC on my laptop(which is my only development environment) because it takes forever to finish.
With something like Github and a CI server(Jenkins/Travis/whatever) integrated to the Github repository that runs tests on pull request, it would be super easy for new contributors to submit small patches.
As far as I can understand(altough currently I can't see how to send a patch) Phabricator helps sending pull requests/patches, but does it help with testing too?
--- Ömer Sinan Ağacan http://osa1.net
-- Regards,
Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/ _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

tl;dir I strongly support this, but for code review only, and only if we can integrate it well with Trac. Phabricator is what we use internally at Facebook, and it's a really good code review tool (better than github, IMO). For one thing, you only get one email for a complete review, rather than one email for each comment(!). The UI is clean and streamlined, and reviewing diffs is a pleasure. If done right, this could improve our workflow a lot. If done wrong, it could just make things more complex - we'd want to steer people away from both github pull requests and attaching patches in Trac and towards Phabricator instead. They have to install a local tool (arc), which could be a barrier to contribution. We would need some integration between Phabricator and Trac. I don't know whether any of this exists, but for example we'd want to have a way to link to Trac tickets from Differential, and to have diffs automatically show up attached to Trac tickets when they are linked to the ticket (by mentioning the ticket in the commit message, for example). So I think this is a lot of work, but if someone were prepared to do it then I think it could improve our lives. Cheers, Simon On 06/06/2014 05:05, Austin Seipp wrote:
Hello all,
Recently, while doing server maintenance, several of the administrators for Haskell.org set up an instance of Phabricator[1], located at https://phabricator.haskell.org
For those who aren't aware, Phabricator (or "Phab") is a suite of tools for software development. Think of it like a polished, semi-private GitHub with a lot of applications and tools for all kinds of needs. We've been using it to do issue tracking for Haskell.org maintenance and like it a lot so far.
One very nice aspect of Phabricator though is it has a very nice code review tool, called 'Differential', that is very useful. For people who have used a tool like Review Board, it's similar. Furthermore, it has a very convenient userland tool called 'Arcanist' which makes it easy for newcomers to post a review and get it merged when it's ready all from the command line.
I'd like to see if people are interested in using Phab _strictly_ for code review of GHC patches. It is a dedicated tool specifically for this, and I think it works much better than Trac or inline GitHub comments.
Also, Phab can also support post-commit reviews. So if I touch something in the runtime system and just push, perhaps Simon or Edward would like to look, and they can be alerted right when I do this, and then yell if I did something stupid.
Before I go much further, I'd like to ask: is there *any* interest in this? Or are people satisifed with Trac? The primary motivations are roughly, in no particular order:
1) Code review is good for everyone, a good way for people to learn the code and ask questions, and useful to give feedback to newcomers. And even experienced GHC hackers can learn things from reading code, as we all do regularly, or find things that need cleanup.
2) Phabricator in particular makes it very easy to submit patches for review. To submit a patch, I just run the command 'arc diff' and it Does The Right Thing. It also makes it easy to ensure people are *alerted* when a patch might be relevant to them.
3) They can be uploaded and created from the command line, and merged easily afterwords the same way. This is particularly useful for newcomers, and for me. :)
4) Differential is dedicated to code review, and much better at it than just reading patches on Trac IMO.
5) It supports both post-commit code review, as well as pre-commit review. Post commit would be especially useful for us too, I think.
Point #2 and #3 are mostly relevant for me, because I mostly handle incoming patches. But I think in general it would be nice, and make it a lot easier for newcomers to submit patches, and us to look over them.
Here's an example of a Differential code review:
https://phabricator.haskell.org/D4
This is a demo using my 'wip/ermsb' patch. You'll need to create an account to login, but it shouldn't be much trouble, you can login several ways. I'll fix the login requirement soon. Feel free to read the code, comment on it, and play around. It's more of a demonstration, but real code review would be welcome too. :)
If people are interested in doing this, I can add notes to the wiki pages for newcomers, and I'll send another email about Phab so people can understand it a little better. But I want to ask first.
There is an argument that our team is so small, code review has unnecessary burdens. But I think Phab could help a lot with tracking outside patches and getting good reviews for incoming patches, and it'll make it easier for newcomers. And experienced pros can probably learn a thing as well.
Again, to be clear, I don't propose we migrate anything to Phabricator from, say, Trac. There's no real pressure to do so and it would be tons of work. I only propose we use it for code review, which is perfectly fine, and how other projects like LLVM do code review (they use Bugzilla).
I also don't think the usage of Phabricator should be mandatory (unless we decide that later because we like it), but I would like to see people use it if possible.

I haven't looked deeply into Trac integration yet, but I believe this
should generally be possible. I'll probably pester the developers
about it later today. I'm glad people seem receptive to it.
I don't think Arcanist will be a barrier, actually. Here's what I
propose: we add arcanist and libphutil to the GHC repository as
submodules (perhaps under ./utils), and just have a top level script
to execute 'arc', e.g. './arc' just executes 'php
./utils/arcanist/bin/arc' or whatnot. Then any user can just checkout
GHC and use the './arc' command to submit diffs - no install
necessary, even on Windows. You'd just need PHP.
This also has the advantage we'll generally want Arcanist to be in
sync for all developers, and roughly in sync with the Phabricator
server. As me and Herbert are admins anyway, keeping the submodule up
to date once every week or two isn't a huge deal.
On Fri, Jun 6, 2014 at 4:54 AM, Simon Marlow
tl;dir I strongly support this, but for code review only, and only if we can integrate it well with Trac.
Phabricator is what we use internally at Facebook, and it's a really good code review tool (better than github, IMO). For one thing, you only get one email for a complete review, rather than one email for each comment(!). The UI is clean and streamlined, and reviewing diffs is a pleasure.
If done right, this could improve our workflow a lot. If done wrong, it could just make things more complex - we'd want to steer people away from both github pull requests and attaching patches in Trac and towards Phabricator instead. They have to install a local tool (arc), which could be a barrier to contribution.
We would need some integration between Phabricator and Trac. I don't know whether any of this exists, but for example we'd want to have a way to link to Trac tickets from Differential, and to have diffs automatically show up attached to Trac tickets when they are linked to the ticket (by mentioning the ticket in the commit message, for example).
So I think this is a lot of work, but if someone were prepared to do it then I think it could improve our lives.
Cheers, Simon
On 06/06/2014 05:05, Austin Seipp wrote:
Hello all,
Recently, while doing server maintenance, several of the administrators for Haskell.org set up an instance of Phabricator[1], located at https://phabricator.haskell.org
For those who aren't aware, Phabricator (or "Phab") is a suite of tools for software development. Think of it like a polished, semi-private GitHub with a lot of applications and tools for all kinds of needs. We've been using it to do issue tracking for Haskell.org maintenance and like it a lot so far.
One very nice aspect of Phabricator though is it has a very nice code review tool, called 'Differential', that is very useful. For people who have used a tool like Review Board, it's similar. Furthermore, it has a very convenient userland tool called 'Arcanist' which makes it easy for newcomers to post a review and get it merged when it's ready all from the command line.
I'd like to see if people are interested in using Phab _strictly_ for code review of GHC patches. It is a dedicated tool specifically for this, and I think it works much better than Trac or inline GitHub comments.
Also, Phab can also support post-commit reviews. So if I touch something in the runtime system and just push, perhaps Simon or Edward would like to look, and they can be alerted right when I do this, and then yell if I did something stupid.
Before I go much further, I'd like to ask: is there *any* interest in this? Or are people satisifed with Trac? The primary motivations are roughly, in no particular order:
1) Code review is good for everyone, a good way for people to learn the code and ask questions, and useful to give feedback to newcomers. And even experienced GHC hackers can learn things from reading code, as we all do regularly, or find things that need cleanup.
2) Phabricator in particular makes it very easy to submit patches for review. To submit a patch, I just run the command 'arc diff' and it Does The Right Thing. It also makes it easy to ensure people are *alerted* when a patch might be relevant to them.
3) They can be uploaded and created from the command line, and merged easily afterwords the same way. This is particularly useful for newcomers, and for me. :)
4) Differential is dedicated to code review, and much better at it than just reading patches on Trac IMO.
5) It supports both post-commit code review, as well as pre-commit review. Post commit would be especially useful for us too, I think.
Point #2 and #3 are mostly relevant for me, because I mostly handle incoming patches. But I think in general it would be nice, and make it a lot easier for newcomers to submit patches, and us to look over them.
Here's an example of a Differential code review:
https://phabricator.haskell.org/D4
This is a demo using my 'wip/ermsb' patch. You'll need to create an account to login, but it shouldn't be much trouble, you can login several ways. I'll fix the login requirement soon. Feel free to read the code, comment on it, and play around. It's more of a demonstration, but real code review would be welcome too. :)
If people are interested in doing this, I can add notes to the wiki pages for newcomers, and I'll send another email about Phab so people can understand it a little better. But I want to ask first.
There is an argument that our team is so small, code review has unnecessary burdens. But I think Phab could help a lot with tracking outside patches and getting good reviews for incoming patches, and it'll make it easier for newcomers. And experienced pros can probably learn a thing as well.
Again, to be clear, I don't propose we migrate anything to Phabricator from, say, Trac. There's no real pressure to do so and it would be tons of work. I only propose we use it for code review, which is perfectly fine, and how other projects like LLVM do code review (they use Bugzilla).
I also don't think the usage of Phabricator should be mandatory (unless we decide that later because we like it), but I would like to see people use it if possible.
-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

I assume if the decission is made to use this, the read-only view won't
require any login or registration?
Nicolas
On Jun 6, 2014 6:05 AM, "Austin Seipp"
Hello all,
Recently, while doing server maintenance, several of the administrators for Haskell.org set up an instance of Phabricator[1], located at https://phabricator.haskell.org
For those who aren't aware, Phabricator (or "Phab") is a suite of tools for software development. Think of it like a polished, semi-private GitHub with a lot of applications and tools for all kinds of needs. We've been using it to do issue tracking for Haskell.org maintenance and like it a lot so far.
One very nice aspect of Phabricator though is it has a very nice code review tool, called 'Differential', that is very useful. For people who have used a tool like Review Board, it's similar. Furthermore, it has a very convenient userland tool called 'Arcanist' which makes it easy for newcomers to post a review and get it merged when it's ready all from the command line.
I'd like to see if people are interested in using Phab _strictly_ for code review of GHC patches. It is a dedicated tool specifically for this, and I think it works much better than Trac or inline GitHub comments.
Also, Phab can also support post-commit reviews. So if I touch something in the runtime system and just push, perhaps Simon or Edward would like to look, and they can be alerted right when I do this, and then yell if I did something stupid.
Before I go much further, I'd like to ask: is there *any* interest in this? Or are people satisifed with Trac? The primary motivations are roughly, in no particular order:
1) Code review is good for everyone, a good way for people to learn the code and ask questions, and useful to give feedback to newcomers. And even experienced GHC hackers can learn things from reading code, as we all do regularly, or find things that need cleanup.
2) Phabricator in particular makes it very easy to submit patches for review. To submit a patch, I just run the command 'arc diff' and it Does The Right Thing. It also makes it easy to ensure people are *alerted* when a patch might be relevant to them.
3) They can be uploaded and created from the command line, and merged easily afterwords the same way. This is particularly useful for newcomers, and for me. :)
4) Differential is dedicated to code review, and much better at it than just reading patches on Trac IMO.
5) It supports both post-commit code review, as well as pre-commit review. Post commit would be especially useful for us too, I think.
Point #2 and #3 are mostly relevant for me, because I mostly handle incoming patches. But I think in general it would be nice, and make it a lot easier for newcomers to submit patches, and us to look over them.
Here's an example of a Differential code review:
https://phabricator.haskell.org/D4
This is a demo using my 'wip/ermsb' patch. You'll need to create an account to login, but it shouldn't be much trouble, you can login several ways. I'll fix the login requirement soon. Feel free to read the code, comment on it, and play around. It's more of a demonstration, but real code review would be welcome too. :)
If people are interested in doing this, I can add notes to the wiki pages for newcomers, and I'll send another email about Phab so people can understand it a little better. But I want to ask first.
There is an argument that our team is so small, code review has unnecessary burdens. But I think Phab could help a lot with tracking outside patches and getting good reviews for incoming patches, and it'll make it easier for newcomers. And experienced pros can probably learn a thing as well.
Again, to be clear, I don't propose we migrate anything to Phabricator from, say, Trac. There's no real pressure to do so and it would be tons of work. I only propose we use it for code review, which is perfectly fine, and how other projects like LLVM do code review (they use Bugzilla).
I also don't think the usage of Phabricator should be mandatory (unless we decide that later because we like it), but I would like to see people use it if possible.
-- Regards,
Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/ _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
participants (12)
-
Arash Rouhani
-
Austin Seipp
-
Carter Schonwald
-
Jan Stolarek
-
Johan Tibell
-
Manuel M T Chakravarty
-
Mateusz Kowalczyk
-
Nicolas Trangez
-
Richard Eisenberg
-
Simon Marlow
-
Simon Peyton Jones
-
Ömer Sinan Ağacan