Re: Phabricator Update, July 2017
On Wednesday, 19 July 2017 16:19:02 UTC-4, Randell Jesup wrote: > >On 2017-07-14 1:31 AM, Jim Blandy wrote: > >> Many people seem to be asking, essentially: What will happen to old bugs? > >> I'm trying to follow the discussion, and I'm not clear on this myself. > >> > >> For example, "Splinter will be turned off." For commenting and reviewing, > >> okay, understood. What about viewing patches on old bugs? > > > >Migration-specific issues are still in flux, since we have still have > >plenty of time to sort them out. I'll be clarifying the migration plan as > >we get closer to turning off old tools. So far we've agreed that keeping > >Splinter in read-only mode to view old patches is totally fine, and Dylan > >(BMO lead) mentioned that there are even nicer diff viewers that we can > >integrate with BMO, like https://diff2html.xyz/, which is currently in use > >by Red Hat's Bugzilla installation. > > Good! your recent posting made me believe you'd gone back to "splinter > must be totally turned off". Thanks. I still have significant > questions about how security-bug access and review (and security of such > items) will work; is there a design? What has changed since our > discussion in May, and have you met with the security engineers and > major security reviewers/triagers? Some platform-security people were presented with the integration plan, but there wasn't much commentary. Most of the discussion has been around implementation details, some of which have been quite tricky. Not much has changed from the original plan except that we are looking into mirroring the CC list over as well as the security groups. Ideally we will have an exact match of the set of users who can view a bug and the set that can view associated revisions. Some of this stuff has already landed on our development instance. We'll have some docs and screencasts to show how it works. Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
>On 2017-07-14 1:31 AM, Jim Blandy wrote: >> Many people seem to be asking, essentially: What will happen to old bugs? >> I'm trying to follow the discussion, and I'm not clear on this myself. >> >> For example, "Splinter will be turned off." For commenting and reviewing, >> okay, understood. What about viewing patches on old bugs? > >Migration-specific issues are still in flux, since we have still have >plenty of time to sort them out. I'll be clarifying the migration plan as >we get closer to turning off old tools. So far we've agreed that keeping >Splinter in read-only mode to view old patches is totally fine, and Dylan >(BMO lead) mentioned that there are even nicer diff viewers that we can >integrate with BMO, like https://diff2html.xyz/, which is currently in use >by Red Hat's Bugzilla installation. Good! your recent posting made me believe you'd gone back to "splinter must be totally turned off". Thanks. I still have significant questions about how security-bug access and review (and security of such items) will work; is there a design? What has changed since our discussion in May, and have you met with the security engineers and major security reviewers/triagers? Thanks -- Randell Jesup, Mozilla Corp remove "news" for personal email ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
>On 7/13/17 9:04 PM, Mark Côté wrote: >> It is also what newer systems >> do today (e.g. GitHub and the full Phabricator suite) > >I should note that with GitHub what this means is that you get discussion >on the PR that should have gone in the issue, with the result that people >following the issue don't see half the relevant discussion. In particular, >it's common to go off from "reviewing this line of code" to "raising >questions about what the desired behavior is", which is squarely back in >issue-land, not code-review land. Yes, exactly. Once the data is in two places, there's no way to avoid some discussions occuring in the "wrong" place. And Github shows it clearly happens all the time, so you still have to flip back and forth between looking at N review-streams of comments, and the bug, and maybe looking at the dates of comments, in order to understand everything. >Unfortunately, I don't know how to solve that problem without designating a >"central point where all discussion happens" and ensuring that anything >outside it is mirrored there... Ditto - I think it's inherent if you can reply to review comments in the separate tool. Making the review-tool *worse* for commenting (features/ease-wise) can actually kinda help, at the cost of hurting responding to review comments. Googe's Issue via codereview tooling kinda takes this approach, from what I've seen, with *very* non-integrated patches vs bugs. GPS's comments about the pain of syncing also makes total sense, in that MozReview tried to mostly attach to existing API points of bugzilla and do things noone anticipated would be done. I don't think limiting comments in phabricator is a viable option, nor making it less-featured, so we may have to eat the pain and lash people with wet noodles if they post in the "wrong" place. -- Randell Jesup, Mozilla Corp remove ".news" for personal email ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Mark Côté wrote: > It was announced in May > (https://groups.google.com/forum/#!topic/mozilla.tools/4qroY2Iia9I), > linked to in this forum: > https://groups.google.com/d/msg/mozilla.dev.platform/qh5scX3Gk2U/xCWe8jrOAQAJ I stand corrected, thanks. I would've thought that'd be put in moz.dev.planning or at least cross-posted since it is a planning issue. Edmund ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Hi Joe, I just want to publicly apologize for being sarcastic in my original post to you. I could've found a better voice and the frustration clouded my judgement. I'm sorry. Edmund ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 2017-07-17 8:46 PM, Edmund Wong wrote: Mike Hoye wrote: Given that we've been talking about this stuff for years now, I think it's very clear that we haven't come to this point by "somebody at the top issuing an edict that they want something modern"; the decision to commit to Phabricator was ultimately announced on May 11th, and that decision wasn't made unilaterally or lightly. It's hardly fair to say that "we've been talking about this stuff for years now" when this is the very first time it's been posted. There was no public notification of the intention of doing away with splinter/MozReview. Sure, there may have been talks but were they public and not just hidden in some obscure thread? It was announced in May (https://groups.google.com/forum/#!topic/mozilla.tools/4qroY2Iia9I), linked to in this forum: https://groups.google.com/d/msg/mozilla.dev.platform/qh5scX3Gk2U/xCWe8jrOAQAJ Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Mon, Jul 17, 2017 at 12:27 PM, Gregory Szorcwrote: > If the bug is only serving as an anchor to track code review, then the > question we should be asking is "do we even need a bug." > In my experience the answer to this is "yes, we need a bug". I very rarely have a one-to-one mapping between bugs and review requests. Sometimes there are many reviews per bug. Sometimes there are no reviews in a bug, but it still contains useful information. All the meta information around bug blockers/dependents is orthogonal to reviews. If you want to get rid of bugzilla in favor of your new tool, you are really creating a new bug tracker with a built-in review tool, IMO. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Mike Hoye wrote: > > Given that we've been talking about this stuff for years now, I think > it's very clear that we haven't come to this point by "somebody at the > top issuing an edict that they want something modern"; the decision to > commit to Phabricator was ultimately announced on May 11th, and that > decision wasn't made unilaterally or lightly. It's hardly fair to say that "we've been talking about this stuff for years now" when this is the very first time it's been posted. There was no public notification of the intention of doing away with splinter/MozReview. Sure, there may have been talks but were they public and not just hidden in some obscure thread? > > We've been discussing how to improve our tooling and processes in open > forums for years now, most frequently on dev-platform. Despite that, > it's true that a lot of the major influences of this decision have been > mostly invisible to people who aren't involved in the day to day work on > infrastructure and tooling, in the way that icebergs are mostly invisible. Not exactly a good analogy there, considering icebergs can also sink ships. So this discussion has been pretty much invisible from people who use the tools and only for the select few. > Some of the things that haven't made it into this thread that have > influenced this decision include: > > - a real and pressing requirement to update our commit access policies > and practices, and backstop them with reliable tooling (a discussion you > participated in, as I recall), Yes. I did participate in that. Though I'm not entirely sure what came off that discussion. > - the significant engineering burdens (including opportunity cost and > bus-factor risk) of being the sole owner/user of a major piece of > infrastructure software that's not our core product, and Oh hell yeah. This truck/bus factor is a definite PITA, and I applaud any effort in fixing this. > - the much more pernicious set of hidden costs we incur from _failing_ > to standardize on certain tools and processes. I agree as well. > > It's a very serious drag on product development and contributor > participation if everyone - paid senior engineers and new contributors > alike - needs to use _this_ tool and process and coding style (and and > and) to participate in one part of the project, and _that_ etc. etc. to > participate in another. Or, sometimes, even other corners of the same > codebase. Oh... hell yeah. You've mentioned quite a lot of pain points in the whole process, and I certainly agree with them. However, it would've been a better process if there was some sort of public declaration of intention of moving to a tool which would streamline the whole review and commit process. While there may have been a smattering of discussions here of commit policies, there wasn't a concise, for lack of a better word, glue to point out that there was going to be this change. The point is this. We have bugzilla and having a review process that enable people to review patches without needing to jump to another tool, would be more effective than needing to jump from one tool (bugzilla) to another one. If Phabricator does the job well, then so be it. Thank you for your clarifications, Mike. Edmund ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
I filed a central tracker bug for production Phabricator deployment: https://bugzilla.mozilla.org/show_bug.cgi?id=1381498. I have filed blockers and dependencies for a variety of related tasks as discussed in these threads. Mark On 2017-07-14 11:33 AM, Milan Sreckovic wrote: Replying in general, to a random message :) I don't have the numbers, but I imagine reviews are happening in the hundreds every day (if we land almost 300 patches.) So, I wouldn't expect the conversation about adding/removing/changing tools involved in reviews to be any less complicated, passionate and involved as what we're having here :) I believe Mark is putting together an e-mail to give us a better place to continue these conversations; something with meta bugs for "enable phabricator", "disable splinter", etc. This should let us track the issues that are raised, discuss decisions as to which of those should be blockers, and have a better record of what actually gets resolved and how. This should leave us *just* with the decision making and communication rollout issues that were raised; I know we are continuously discussing those and trying to get better at it, but it is a separate conversation from the technical issues, so it's probably OK that it remains separate. On 13-Jul-17 15:41, Randell Jesup wrote: To answer the other part of your question, MozReview will be disabled for active use across the board, but it is currently used by a small number of projects. Splinter will be disabled on a per-product basis, as there may be some projects that can't, won't, or shouldn't be migrated to Phabricator. Splinter is still a nice UI to look at patches already attached to bugs. Please don't disable it. excellent point; thanks for that feedback. instead of disabling splinter for phabricator backed products, we could make it a read-only patch viewer. I would consider it a massive fail if we disabled at least read-only splinter viewing of patches. As noted before. let's make sure we don't lose things we agreed on as issues. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 7/16/17 11:10 PM, Edmund Wong wrote: Joe Hildebrand wrote: I'm responding at the top of the thread here so that I'm not singling out any particular response. We didn't make clear in this process how much work Mark and his team did ahead of the decision to gather feedback from senior engineers on both Selena and my teams, and how deeply committed the directors have been in their support of this change. Really? So? What exactly is your point? This sarcasm is unwarranted. Given that we've been talking about this stuff for years now, I think it's very clear that we haven't come to this point by "somebody at the top issuing an edict that they want something modern"; the decision to commit to Phabricator was ultimately announced on May 11th, and that decision wasn't made unilaterally or lightly. We've been discussing how to improve our tooling and processes in open forums for years now, most frequently on dev-platform. Despite that, it's true that a lot of the major influences of this decision have been mostly invisible to people who aren't involved in the day to day work on infrastructure and tooling, in the way that icebergs are mostly invisible. Some of the things that haven't made it into this thread that have influenced this decision include: - a real and pressing requirement to update our commit access policies and practices, and backstop them with reliable tooling (a discussion you participated in, as I recall), - the significant engineering burdens (including opportunity cost and bus-factor risk) of being the sole owner/user of a major piece of infrastructure software that's not our core product, and - the much more pernicious set of hidden costs we incur from _failing_ to standardize on certain tools and processes. It's a very serious drag on product development and contributor participation if everyone - paid senior engineers and new contributors alike - needs to use _this_ tool and process and coding style (and and and) to participate in one part of the project, and _that_ etc. etc. to participate in another. Or, sometimes, even other corners of the same codebase. Each of those on their own is a huge deal, probably big enough to warrant a major change in our tooling and processes. And it's true that we haven't done a great job of explicitly spelling out what a big deal they are, and how much they've informed our decision-making. But while we support - and will always support - user choice in our products, there aren't enough engineering hours in a year for us to support a bunch of different ways of shipping, securing, measuring and improving the tools and processes that provide those choices _to_ our users. So, yes, we could have done a better job of communicating here and making our process more transparent, but I'm very confident that this decision will make life a lot better for Mozilla's casual contributors and broader community as much as for Mozilla's full-time engineers. If you'd like to dig into the details of "why" I'd be happy to do so. And if we can do that without unnecessary snark, all the better. - mhoye ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
> On Jul 15, 2017, at 23:36, Gabriele Sveltowrote: > >> On 14/07/2017 05:39, Boris Zbarsky wrote: >> I should note that with GitHub what this means is that you get >> discussion on the PR that should have gone in the issue, with the result >> that people following the issue don't see half the relevant discussion. >> In particular, it's common to go off from "reviewing this line of code" >> to "raising questions about what the desired behavior is", which is >> squarely back in issue-land, not code-review land. >> >> Unfortunately, I don't know how to solve that problem without >> designating a "central point where all discussion happens" and ensuring >> that anything outside it is mirrored there... > > Yeah, we frequently had that problem with Gaia issues as part of Firefox > OS. Reviews were done on GitHub's PR so the bugzilla entries were often > empty (even for bugs with huge, complex patch-sets). To gather any kind > of information one had to go and check the comments on the PR itself > which not only was less-than-optimal but made life a lot harder for > those not directly involved with development. If the bug is only serving as an anchor to track code review, then the question we should be asking is "do we even need a bug." I've explored this a bit in https://gregoryszorc.com/blog/2015/01/16/bugzilla-and-the-future-of-firefox-development/. That post was written 2.5 years ago. If you replace "MozReview" with "Phabricator," many of the points I made are still relevant. We're just taking a drastically different path. Also, it was written very early in MozReview's development and reflected more of a long term workflow we wanted to enable. It became apparent that many things would not be easily achievable or were not reasonable at that time. That's why we dropped/paused many of the more drastic ideas. Making the decision to decouple review from issue tracking with Phabricator brings many of them back to the table. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Thursday, July 13, 2017 at 11:39:38 PM UTC-4, Boris Zbarsky wrote: > On 7/13/17 9:04 PM, Mark Côté wrote: > > It is also what newer systems > > do today (e.g. GitHub and the full Phabricator suite) > > I should note that with GitHub what this means is that you get > discussion on the PR that should have gone in the issue, with the result > that people following the issue don't see half the relevant discussion. > In particular, it's common to go off from "reviewing this line of code" > to "raising questions about what the desired behavior is", which is > squarely back in issue-land, not code-review land. > > Unfortunately, I don't know how to solve that problem without > designating a "central point where all discussion happens" and ensuring > that anything outside it is mirrored there... So I for one, tend to like to read commentary on interesting bugs inline in my e-mail with said bug, so that I can get some context for the "why" along with "patch created" --> "comment on patch" --> "[context of patch] why did this line change" ---> ! Aha, I can help with context there! (or Maybe changing that hunk of that file is interesting to me and I can provide useful feedback). So, a few concrete suggestions on how I imagine we can meet my desire of having patch comment/review/etc in bug, without having the downsides called out in this thread (no deep thought on difficulty of implementation for these ideas, I trust those reading to evaluate utility against the difficulty): * Comments on patches get emailed to the people in the bug contact lists (using similar X-Bugzilla Headers) * Comments/Changes in phabricator get viewable in bugzilla directly. - Patch Creation/Obsolescence (options below) * New patches in patch list, like mozreview/traditional * An integrated table that merely queries a phabricator API * A bmo comment * An entry inline with bmo comments to indicate phabricator patch created/obsoleted, but without being part of bmo in any other way. - Patch flag setting (Depends partly on solution(s) chosen above) * Actual bmo flag flipped * Table of phabricator reviews updated * A bmo comment saying state of review changed * An entry inline with bmo comments but not actually a comment - Patch commentary * Actual BMO comment * A `phabricator` comment viewed in bmo (iframe/direct paste/whatever) as a read-only thing * A BMO comment but `reply` opens in phabricator * A BMO comment that says "review [x] updated in phabricator: [link]" * A inline history entry showing that a review comment happened in phabricator, with link. * ...etc? The bottom line for me, is that I'd like to be able to see, at a glance, that "stuff" happened in the review tool when I'm looking at a bug in bmo. There are various degrees of this that could make me happier than unhappy, though I'm aware that some of my own desires conflict directly with desires of others, and in relative difficulty to implement/maintain. I'd love to hear the outcome of this subthread, regardless of what choice (if any) of what I proposed, is made. ~Justin Wood (Callek) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Joe Hildebrand wrote: > I'm responding at the top of the thread here so that I'm not singling out any > particular response. > > We didn't make clear in this process how much work Mark and his team did > ahead of the decision to gather feedback from senior engineers on both Selena > and my teams, and how deeply committed the directors have been in their > support of this change. Really? So? What exactly is your point? 1) That Mark and his team put a lot of effort? 2) You gathered feedback from senior engineers? 3) The directors are deeply committed with this change? So you've asked senior engineers because they do the brunt of the reviews. Is this the new transparent way about selecting a tool which *everyone* uses and not just the selected few? [No... I'm not complaining because I wasn't selected. I'm complaining because Mozilla is doing a pdf.js with Splinter.] Don't get me wrong. I have absolutely no doubt Mark and his team placed a lot of effort in setting this up. Just as a lot of effort was put behind MozReview and Splinter. But the thing is, when someone at the top issues an edict that they want something 'modern', someone has to put the effort in, so saying what you said is just a red herring. It doesn't explain the rationale or even the criteria in selecting the new tool. > > Seeing a need for more modern patch reviewing at Mozilla, Laura Thomson's > team did an independent analysis of the most popular tools available on the market today. Working with the NSS team to pilot What exactly does a "more modern patch reviewing" mean? What is the set of criteria that would deem a tool to be 'modern'? All I am seeing is what I saw with PDF.js; that is, Mozilla is moving away from in-house solutions to external ones. Fine. That is Moco's choice. And yes, I have used Phabricator before so I know what it looks like. I'm just disagreeing with the necessity of setting up an external tool to Bugzilla's Splinter. But I suppose, I'll wait and see how this pans out. Perhaps two years down the road, something more 'modern' will come along and we'll go through this again. A company such as Mozilla that's purporting to support choice is certainly not really practicing what it preaches. In this specific example, we have seen Mozilla break the 8th edict of the Mozilla Manifesto; that is: "Transparent community-based processes promote participation, accountability and trust." The choice of Phabricator was neither transparent, nor community-based. So how is this going to promote participation, accountability and trust? Good luck with that. Again... I will wait and see how this Phabricator pans out. Worse comes to worse, I just copy and paste the code and review it in the comment section. > Therefore, I would appreciate that if you feel the need to further comment on > this thread, we focus on what can be done to make this transition successful, > rather than appearing to be standing in the way of progress. "standing in the way of progress"? Ouch. That is really harsh. Has that "if you're not with us, you're against us" type of vibe. Seriously... was that even necessary? Well... if that's the case, I'll just wait until gets released and have a go again. Edmund ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 14/07/2017 05:39, Boris Zbarsky wrote: > I should note that with GitHub what this means is that you get > discussion on the PR that should have gone in the issue, with the result > that people following the issue don't see half the relevant discussion. > In particular, it's common to go off from "reviewing this line of code" > to "raising questions about what the desired behavior is", which is > squarely back in issue-land, not code-review land. > > Unfortunately, I don't know how to solve that problem without > designating a "central point where all discussion happens" and ensuring > that anything outside it is mirrored there... Yeah, we frequently had that problem with Gaia issues as part of Firefox OS. Reviews were done on GitHub's PR so the bugzilla entries were often empty (even for bugs with huge, complex patch-sets). To gather any kind of information one had to go and check the comments on the PR itself which not only was less-than-optimal but made life a lot harder for those not directly involved with development. It's not a dealbreaker for me but it's something that should be kept in mind. Gabriele signature.asc Description: OpenPGP digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Jul 14, 2017 6:27 PM, "Mike Hommey"wrote: On Fri, Jul 14, 2017 at 01:00:51PM -0400, Ben Kelly wrote: > I know feedback was collected, but maybe not from this group. Feedback was collected from a selected set of the people who do the most reviews. I'm one of them. I don't think I'm breaking any secret by saying that the list of people who received the consultation email at the same time as I did were maire, mconley, bz, ehsan, smaug, billm and mattn. That's good! That was a generic consultation as to what a review tool should provide or not, before any decision was made on a specific product. I guess there should be another round for implementation details, if there isn't one already. Yea, getting buy-in and consensus for a solution from those heavy reviewers would probably increase the chance of success. Thanks. Ben ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Fri, Jul 14, 2017 at 01:00:51PM -0400, Ben Kelly wrote: > Also a random reply. > > I think this kind of effort is more likely to be successful if it gets > input and buy-in from the key stakeholders. In this case that would be the > most frequent reviewers. > > It would be nice to run a bugzilla query to find the top 10 or 20 > reviewers. Talk to these folks, solve their problems, and get buy-in for a > solution. The rest of the org will likely follow their lead. > > Right now, though, I talk to people who spend all day reviewing and they > tell me no one ever talked to them. > > I know feedback was collected, but maybe not from this group. Feedback was collected from a selected set of the people who do the most reviews. I'm one of them. I don't think I'm breaking any secret by saying that the list of people who received the consultation email at the same time as I did were maire, mconley, bz, ehsan, smaug, billm and mattn. That was a generic consultation as to what a review tool should provide or not, before any decision was made on a specific product. I guess there should be another round for implementation details, if there isn't one already. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Also a random reply. I think this kind of effort is more likely to be successful if it gets input and buy-in from the key stakeholders. In this case that would be the most frequent reviewers. It would be nice to run a bugzilla query to find the top 10 or 20 reviewers. Talk to these folks, solve their problems, and get buy-in for a solution. The rest of the org will likely follow their lead. Right now, though, I talk to people who spend all day reviewing and they tell me no one ever talked to them. I know feedback was collected, but maybe not from this group. Anyway, just a suggestion. Ben On Jul 14, 2017 11:34 AM, "Milan Sreckovic"wrote: > Replying in general, to a random message :) > > I don't have the numbers, but I imagine reviews are happening in the > hundreds every day (if we land almost 300 patches.) So, I wouldn't expect > the conversation about adding/removing/changing tools involved in reviews > to be any less complicated, passionate and involved as what we're having > here :) > > I believe Mark is putting together an e-mail to give us a better place to > continue these conversations; something with meta bugs for "enable > phabricator", "disable splinter", etc. This should let us track the issues > that are raised, discuss decisions as to which of those should be blockers, > and have a better record of what actually gets resolved and how. > > This should leave us *just* with the decision making and communication > rollout issues that were raised; I know we are continuously discussing > those and trying to get better at it, but it is a separate conversation > from the technical issues, so it's probably OK that it remains separate. > > > On 13-Jul-17 15:41, Randell Jesup wrote: > >> To answer the other part of your question, MozReview will be disabled for > active use across the board, but it is currently used by a small > number of > projects. Splinter will be disabled on a per-product basis, as there > may be > some projects that can't, won't, or shouldn't be migrated to > Phabricator. > Splinter is still a nice UI to look at patches already attached to bugs. Please don't disable it. >>> excellent point; thanks for that feedback. >>> >>> instead of disabling splinter for phabricator backed products, we could >>> make it a read-only patch viewer. >>> >> I would consider it a massive fail if we disabled at least read-only >> splinter viewing of patches. As noted before. let's make sure we >> don't lose things we agreed on as issues. >> >> > -- > - Milan (mi...@mozilla.com) > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Replying in general, to a random message :) I don't have the numbers, but I imagine reviews are happening in the hundreds every day (if we land almost 300 patches.) So, I wouldn't expect the conversation about adding/removing/changing tools involved in reviews to be any less complicated, passionate and involved as what we're having here :) I believe Mark is putting together an e-mail to give us a better place to continue these conversations; something with meta bugs for "enable phabricator", "disable splinter", etc. This should let us track the issues that are raised, discuss decisions as to which of those should be blockers, and have a better record of what actually gets resolved and how. This should leave us *just* with the decision making and communication rollout issues that were raised; I know we are continuously discussing those and trying to get better at it, but it is a separate conversation from the technical issues, so it's probably OK that it remains separate. On 13-Jul-17 15:41, Randell Jesup wrote: To answer the other part of your question, MozReview will be disabled for active use across the board, but it is currently used by a small number of projects. Splinter will be disabled on a per-product basis, as there may be some projects that can't, won't, or shouldn't be migrated to Phabricator. Splinter is still a nice UI to look at patches already attached to bugs. Please don't disable it. excellent point; thanks for that feedback. instead of disabling splinter for phabricator backed products, we could make it a read-only patch viewer. I would consider it a massive fail if we disabled at least read-only splinter viewing of patches. As noted before. let's make sure we don't lose things we agreed on as issues. -- - Milan (mi...@mozilla.com) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 2017-07-14 1:31 AM, Jim Blandy wrote: Many people seem to be asking, essentially: What will happen to old bugs? I'm trying to follow the discussion, and I'm not clear on this myself. For example, "Splinter will be turned off." For commenting and reviewing, okay, understood. What about viewing patches on old bugs? Migration-specific issues are still in flux, since we have still have plenty of time to sort them out. I'll be clarifying the migration plan as we get closer to turning off old tools. So far we've agreed that keeping Splinter in read-only mode to view old patches is totally fine, and Dylan (BMO lead) mentioned that there are even nicer diff viewers that we can integrate with BMO, like https://diff2html.xyz/, which is currently in use by Red Hat's Bugzilla installation. Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
I'm > On 14 Jul 2017, at 11:39, Boris Zbarskywrote: > >> On 7/13/17 9:04 PM, Mark Côté wrote: >> It is also what newer systems >> do today (e.g. GitHub and the full Phabricator suite) > > I should note that with GitHub what this means is that you get discussion on > the PR that should have gone in the issue, with the result that people > following the issue don't see half the relevant discussion. In particular, > it's common to go off from "reviewing this line of code" to "raising > questions about what the desired behavior is", which is squarely back in > issue-land, not code-review land. > > Unfortunately, I don't know how to solve that problem without designating a > "central point where all discussion happens" and ensuring that anything > outside it is mirrored there... > > -Boris > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
lets all try I'm > On 14 Jul 2017, at 13:48, Jim Blandywrote: > > Yeah, this is kind of the opposite of "No New Rationale". > > https://air.mozilla.org/friday-plenary-rust-and-the-community/ > > >> On Thu, Jul 13, 2017 at 2:49 PM, David Anderson wrote: >> >>> On Thursday, July 13, 2017 at 1:38:18 PM UTC-7, Joe Hildebrand wrote: >>> I'm responding at the top of the thread here so that I'm not singling >> out any particular response. >>> >>> We didn't make clear in this process how much work Mark and his team did >> ahead of the decision to gather feedback from senior engineers on both >> Selena and my teams, and how deeply committed the directors have been in >> their support of this change. >>> >>> Seeing a need for more modern patch reviewing at Mozilla, Laura >> Thomson's team did an independent analysis of the most popular tools >> available on the market today. Working with the NSS team to pilot their >> selected tool, they found that Phabricator is a good fit for our >> development approach (coincidentally a good enough fit that the Graphics >> team was also piloting Phabricator in parallel). After getting the support >> of the Engineering directors, they gathered feedback from senior engineers >> and managers on the suggested approach, and tweaked dates and features to >> match up with our release cycles more appropriately. >> >> The problem is this hasn't been transparent. It was announced as an edict, >> and I don't remember seeing any public discussion beforehand. If senior >> engineers were consulted, it wasn't many of them - and the only person I >> know who was, was consulted after the decision was made. >> >> I've contributed thousands of patches over many years and haven't really >> seen an explanation of how this change will make my development process >> easier. Maybe it will, or maybe it won't. It probably won't be a big deal >> because this kind of tooling is not really what makes development hard. I >> don't spend most of my day figuring out how to get a changeset from one >> place to another. >> >> The fact is that no one really asked us beforehand, "What would make >> development easier?" We're just being told that Phabricator will. That's >> why people are skeptical. >> ___ >> dev-platform mailing list >> dev-platform@lists.mozilla.org >> https://lists.mozilla.org/listinfo/dev-platform >> > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 7/14/17 1:31 AM, Jim Blandy wrote: But keeping all the comments in one thread is a mixed blessing, too Absolutely. I guess what I'm saying is we should try to have some guidelines for when it's appropriate to take the discussion back to the bug instead of continuing it in the review... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Yeah, this is kind of the opposite of "No New Rationale". https://air.mozilla.org/friday-plenary-rust-and-the-community/ On Thu, Jul 13, 2017 at 2:49 PM, David Andersonwrote: > On Thursday, July 13, 2017 at 1:38:18 PM UTC-7, Joe Hildebrand wrote: > > I'm responding at the top of the thread here so that I'm not singling > out any particular response. > > > > We didn't make clear in this process how much work Mark and his team did > ahead of the decision to gather feedback from senior engineers on both > Selena and my teams, and how deeply committed the directors have been in > their support of this change. > > > > Seeing a need for more modern patch reviewing at Mozilla, Laura > Thomson's team did an independent analysis of the most popular tools > available on the market today. Working with the NSS team to pilot their > selected tool, they found that Phabricator is a good fit for our > development approach (coincidentally a good enough fit that the Graphics > team was also piloting Phabricator in parallel). After getting the support > of the Engineering directors, they gathered feedback from senior engineers > and managers on the suggested approach, and tweaked dates and features to > match up with our release cycles more appropriately. > > The problem is this hasn't been transparent. It was announced as an edict, > and I don't remember seeing any public discussion beforehand. If senior > engineers were consulted, it wasn't many of them - and the only person I > know who was, was consulted after the decision was made. > > I've contributed thousands of patches over many years and haven't really > seen an explanation of how this change will make my development process > easier. Maybe it will, or maybe it won't. It probably won't be a big deal > because this kind of tooling is not really what makes development hard. I > don't spend most of my day figuring out how to get a changeset from one > place to another. > > The fact is that no one really asked us beforehand, "What would make > development easier?" We're just being told that Phabricator will. That's > why people are skeptical. > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Many people seem to be asking, essentially: What will happen to old bugs? I'm trying to follow the discussion, and I'm not clear on this myself. For example, "Splinter will be turned off." For commenting and reviewing, okay, understood. What about viewing patches on old bugs? Yes, Phabricator will segregate review comments and general discussion. But keeping all the comments in one thread is a mixed blessing, too; for example, check out bug 1271650 and try to tell me what the status of each patch is. "Well, those patches should be split across several bugs." Okay, but that fragments the conversation too. There's an inherent tension here, and Ye Olde Bugzilla Patch Review Process is more of a lovingly polished turd than a good solution. On Thu, Jul 13, 2017 at 8:39 PM, Boris Zbarskywrote: > On 7/13/17 9:04 PM, Mark Côté wrote: > >> It is also what newer systems >> do today (e.g. GitHub and the full Phabricator suite) >> > > I should note that with GitHub what this means is that you get discussion > on the PR that should have gone in the issue, with the result that people > following the issue don't see half the relevant discussion. In particular, > it's common to go off from "reviewing this line of code" to "raising > questions about what the desired behavior is", which is squarely back in > issue-land, not code-review land. > > Unfortunately, I don't know how to solve that problem without designating > a "central point where all discussion happens" and ensuring that anything > outside it is mirrored there... > > -Boris > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 7/13/17 9:04 PM, Mark Côté wrote: It is also what newer systems do today (e.g. GitHub and the full Phabricator suite) I should note that with GitHub what this means is that you get discussion on the PR that should have gone in the issue, with the result that people following the issue don't see half the relevant discussion. In particular, it's common to go off from "reviewing this line of code" to "raising questions about what the desired behavior is", which is squarely back in issue-land, not code-review land. Unfortunately, I don't know how to solve that problem without designating a "central point where all discussion happens" and ensuring that anything outside it is mirrored there... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Thu, Jul 13, 2017 at 6:04 PM, Mark Côtéwrote: > On 2017-07-13 3:54 PM, Randell Jesup wrote: > >> On Wed, Jul 12, 2017 at 11:27 AM, Byron Jones wrote: >>> >>> But indeed having also the patches in bugzilla would be good. > > no, it would be bad for patches to be duplicated into bugzilla. we're moving from bugzilla/mozreview to phabricator for code review, duplicating phabricate reviews back into the old systems seems like a step backwards (and i'm not even sure what the value is of doing so). >>> >>> I find this a strange argument. We don't know how successful >>> phrabricator >>> is going to be. The last attempt to switch review tools seems to be >>> getting killed. I think its somewhat reasonable to be skeptical of >>> further >>> review tool changes. >>> >> >> I quite agree... And I hate the idea of having the data spread across >> systems (which I also disliked in MozReview, which I tried and it ended >> up being really bad for some aspects of my ability to land patches). >> > > Mirroring comments from Phabricator to BMO is even more confusing, as we > end up with forked discussions, where some people reply only on BMO. This > happens regularly with MozReview, as a quick survey of recently fixed bugs > shows. Keeping code review in one place, loosely coupled to issue > tracking, improves readability of bugs by keeping discussion of issues > separate from specific solutions. It is also what newer systems do today > (e.g. GitHub and the full Phabricator suite), which I mention not as a > reason in and of itself but to note precedent. The plan is to move to a > separate, more modern, and more powerful code-review tool. Forked > discussions means that the bugs will always have to be consulted for code > reviews to progress, which undermines the utility of the code-review tool > itself. Loose coupling also frees us to try experiments like patches > without bugs, which has been discussed many times but has been technically > blocked from implementation. > > That said, lightweight linkages between issues and code review are very > useful. We're doing some of that, and we're open to iterating more there. > > We've been asked to be bold and innovate all across Mozilla, to try new > things and see if they work. For a long time, Mozillians have discussed > modernized workflows, with new tools that also open more avenues to > automation, but such things have been rarely attempted, and even then in a > disorganized fashion. We're finally at a time when we have the ability and > support to forge ahead, and that's what we're doing. > This. As someone who worked on MozReview, I can say unequivocally that one of the hardest parts - and I dare say the thing that held MozReview back the most - was the tight coupling with Bugzilla and the confusion and complexity that arose from it. When we deployed MozReview, we intentionally tried to not rock the boat too hard: we wanted it to be an extension of the Bugzilla-centric workflow that everyone was familiar with. Was there some boat rocking, yes: that's the nature of change. But we went out of our way to accommodate familiar workflows and tried to find the right balance between old and new. This resulted in obvious awkwardness, like trying to map ReviewBoard's "Ship It" field to Bugzilla's complicated review flag mechanism. Does a review that doesn't grant "Ship It" clear the r? flag or leave it? What happens when someone grants "Ship It" but they don't have permissions to leave review flags in Bugzilla? How do you convey r-? What about feedback flags? What happens when someone changes a review flag in Bugzilla? Then you have other awkwardness, like when you mirror the review comment to Bugzilla and it loses rich text formatting. Or someone replies to a MozReview comment on Bugzilla and you can't see that reply on MozReview. Do you disable replying to comments mirrored from MozReview? That's annoying. Do you disable the mirroring then? That's annoying too! Bi-directional sync? No way! (If you've ever implemented bi-directional sync you'll know why.) You just can't win. The usability issues stemming from trying to overlay ReviewBoard's and Bugzilla's models of how the world worked were obvious and frustrating. It took months to years to iron things out. And we arguably never got it right. Then there were technical issues. When you did things like post a series of commits to review on MozReview, this was done so as an atomic operation via a database transaction. It either worked or it didn't. But we had to mirror those reviews to Bugzilla. Bugzilla didn't have an API to atomically create multiple attachments/reviews. So not only was the publishing to Bugzilla slow because of multiple HTTP requests, but it was also non-atomic. When Bugzilla randomly failed (all HTTP requests randomly fail: it is a property of networks), the review series in Bugzilla was sometimes left in an
Re: Phabricator Update, July 2017
On 2017-07-13 3:54 PM, Randell Jesup wrote: On Wed, Jul 12, 2017 at 11:27 AM, Byron Joneswrote: But indeed having also the patches in bugzilla would be good. no, it would be bad for patches to be duplicated into bugzilla. we're moving from bugzilla/mozreview to phabricator for code review, duplicating phabricate reviews back into the old systems seems like a step backwards (and i'm not even sure what the value is of doing so). I find this a strange argument. We don't know how successful phrabricator is going to be. The last attempt to switch review tools seems to be getting killed. I think its somewhat reasonable to be skeptical of further review tool changes. I quite agree... And I hate the idea of having the data spread across systems (which I also disliked in MozReview, which I tried and it ended up being really bad for some aspects of my ability to land patches). Mirroring comments from Phabricator to BMO is even more confusing, as we end up with forked discussions, where some people reply only on BMO. This happens regularly with MozReview, as a quick survey of recently fixed bugs shows. Keeping code review in one place, loosely coupled to issue tracking, improves readability of bugs by keeping discussion of issues separate from specific solutions. It is also what newer systems do today (e.g. GitHub and the full Phabricator suite), which I mention not as a reason in and of itself but to note precedent. The plan is to move to a separate, more modern, and more powerful code-review tool. Forked discussions means that the bugs will always have to be consulted for code reviews to progress, which undermines the utility of the code-review tool itself. Loose coupling also frees us to try experiments like patches without bugs, which has been discussed many times but has been technically blocked from implementation. That said, lightweight linkages between issues and code review are very useful. We're doing some of that, and we're open to iterating more there. We've been asked to be bold and innovate all across Mozilla, to try new things and see if they work. For a long time, Mozillians have discussed modernized workflows, with new tools that also open more avenues to automation, but such things have been rarely attempted, and even then in a disorganized fashion. We're finally at a time when we have the ability and support to forge ahead, and that's what we're doing. Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Thursday, July 13, 2017 at 1:38:18 PM UTC-7, Joe Hildebrand wrote: > I'm responding at the top of the thread here so that I'm not singling out any > particular response. > > We didn't make clear in this process how much work Mark and his team did > ahead of the decision to gather feedback from senior engineers on both Selena > and my teams, and how deeply committed the directors have been in their > support of this change. > > Seeing a need for more modern patch reviewing at Mozilla, Laura Thomson's > team did an independent analysis of the most popular tools available on the > market today. Working with the NSS team to pilot their selected tool, they > found that Phabricator is a good fit for our development approach > (coincidentally a good enough fit that the Graphics team was also piloting > Phabricator in parallel). After getting the support of the Engineering > directors, they gathered feedback from senior engineers and managers on the > suggested approach, and tweaked dates and features to match up with our > release cycles more appropriately. The problem is this hasn't been transparent. It was announced as an edict, and I don't remember seeing any public discussion beforehand. If senior engineers were consulted, it wasn't many of them - and the only person I know who was, was consulted after the decision was made. I've contributed thousands of patches over many years and haven't really seen an explanation of how this change will make my development process easier. Maybe it will, or maybe it won't. It probably won't be a big deal because this kind of tooling is not really what makes development hard. I don't spend most of my day figuring out how to get a changeset from one place to another. The fact is that no one really asked us beforehand, "What would make development easier?" We're just being told that Phabricator will. That's why people are skeptical. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
I'm responding at the top of the thread here so that I'm not singling out any particular response. We didn't make clear in this process how much work Mark and his team did ahead of the decision to gather feedback from senior engineers on both Selena and my teams, and how deeply committed the directors have been in their support of this change. Seeing a need for more modern patch reviewing at Mozilla, Laura Thomson's team did an independent analysis of the most popular tools available on the market today. Working with the NSS team to pilot their selected tool, they found that Phabricator is a good fit for our development approach (coincidentally a good enough fit that the Graphics team was also piloting Phabricator in parallel). After getting the support of the Engineering directors, they gathered feedback from senior engineers and managers on the suggested approach, and tweaked dates and features to match up with our release cycles more appropriately. Although there may be other risks that weren't identified in our approach, I'm personally satisfied that what we have in front of us is strictly better than where we are today. Of course, we'll have to sand and fill a little bit to perfect the new Phabricator approach, and deal with our transition away from existing systems such as Splinter and MozReview. However, that tweaking would be required no matter what tool we chose, or when we chose to make a change. Therefore, I would appreciate that if you feel the need to further comment on this thread, we focus on what can be done to make this transition successful, rather than appearing to be standing in the way of progress. For example, "I'm concerned about X; I wonder if we could do Y to mitigate that concern?" is a much more powerful approach than "X!" at this point. — Joe Hildebrand > On Jul 11, 2017, at 2:41 PM, Mark Côtéwrote: > > Hi all, here's a brief update on the project to deploy and integrate > Phabricator at Mozilla: > > * Development Phabricator instance is up at > https://mozphab.dev.mozaws.net/, authenticated via > bugzilla-dev.allizom.org. > > * Development, read-only UI for Lando (the new automatic-landing > service) has been deployed. > > * Work is proceeding on matching viewing restrictions on Phabricator > revisions (review requests) to associated confidential bugs. > > * Work is proceeding on the internals of Lando to land Phabricator > revisions to the autoland Mercurial branch. > > * Pre-release of Phabricator, without Lando, targeted for mid-August. > > * General release of Phabricator and Lando targeted for late September or > early October. > > * MozReview and Splinter turned off in early December. > > I have a blog post up with many more details: > https://mrcote.info/blog/2017/07/11/phabricator-update/ > > More to come! > > Mark > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
>On Wed, Jul 12, 2017 at 11:27 AM, Byron Joneswrote: > >> But indeed having also the patches in bugzilla would be good. >>> >> no, it would be bad for patches to be duplicated into bugzilla. we're >> moving from bugzilla/mozreview to phabricator for code review, duplicating >> phabricate reviews back into the old systems seems like a step backwards >> (and i'm not even sure what the value is of doing so). > >I find this a strange argument. We don't know how successful phrabricator >is going to be. The last attempt to switch review tools seems to be >getting killed. I think its somewhat reasonable to be skeptical of further >review tool changes. I quite agree... And I hate the idea of having the data spread across systems (which I also disliked in MozReview, which I tried and it ended up being really bad for some aspects of my ability to land patches). >Also, I have to say the whole phabricator thing has been kind of presented >as a fait accompli. As someone who continues to use splinter on a daily >basis I'm 1) skeptical and 2) kind of unhappy. > >Maybe phabricator will be great, but I'll believe it when I see it. Please >don't force future data and workflow into an as-yet unproven tool. This while sequence strikes me as "Let's launch our new boat, and oh by the way we're going to sink the old boat(s) so no one is tempted to not move to the new boat. And, BTW, we're leaving behind some important things, and we'll think about how we can deal with those sometime, maybe before we launch, maybe later, maybe never." While this may force people onto the new boat, I'm seriously unconvinced this will be better in the short term, or that it won't end up slowing down N*100 developers, either for a while or permanently. And the lack of transparency in developing this plan is also very concerning. IMO -- Randell Jesup, Mozilla Corp remove "news" for personal email ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
>>> To answer the other part of your question, MozReview will be disabled for >>> active use across the board, but it is currently used by a small number of >>> projects. Splinter will be disabled on a per-product basis, as there may be >>> some projects that can't, won't, or shouldn't be migrated to Phabricator. >> >> Splinter is still a nice UI to look at patches already attached to bugs. >> Please don't disable it. > >excellent point; thanks for that feedback. > >instead of disabling splinter for phabricator backed products, we could >make it a read-only patch viewer. I would consider it a massive fail if we disabled at least read-only splinter viewing of patches. As noted before. let's make sure we don't lose things we agreed on as issues. -- Randell Jesup, Mozilla Corp remove "news" for personal email ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Wed, Jul 12, 2017 at 11:54 AM, Byron Joneswrote: > >> Consider that we are talking about "turning off" mozreview now. Will all >> the bugzilla links to those reviews go dead? Or do we have to maintain a >> second service in read-only mode forever? > > the patches will be archived in some form. > > how this looks is yet to be fully fleshed out - ideas currently include > archiving and updating bugzilla to point to their new location (eg. s3), or > uploading patches directly to bugzilla. Note that merely uploading a MozReview patch to bugzilla potentially loses some information: in MozReview, all of the patch's ancestors were available in a repository such that you could pull the patch and apply it locally without having to rebase and potentially encounter conflicts. That's not true of plain patches uploaded to bugzilla. Cheers, Botond ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 7/12/17 11:54 AM, Byron Jones wrote: or uploading patches directly to bugzilla. But still rewriting existing links (including from the mirrored review comment comments, so it's clear which diff the review comments applied to), right? -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Wed, Jul 12, 2017 at 8:54 AM, Byron Joneswrote: > Consider that we are talking about "turning off" mozreview now. Will all >> the bugzilla links to those reviews go dead? Or do we have to maintain a >> second service in read-only mode forever? >> > > the patches will be archived in some form. > The final patches are available in the source repo. Will you also include the interim versions and the important review comments that are sometimes needed to understand a particular change was made? - Dan Veditz ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Wed, Jul 12, 2017 at 11:27 AM, Byron Joneswrote: > But indeed having also the patches in bugzilla would be good. >> > no, it would be bad for patches to be duplicated into bugzilla. we're > moving from bugzilla/mozreview to phabricator for code review, duplicating > phabricate reviews back into the old systems seems like a step backwards > (and i'm not even sure what the value is of doing so). > I find this a strange argument. We don't know how successful phrabricator is going to be. The last attempt to switch review tools seems to be getting killed. I think its somewhat reasonable to be skeptical of further review tool changes. Also, I have to say the whole phabricator thing has been kind of presented as a fait accompli. As someone who continues to use splinter on a daily basis I'm 1) skeptical and 2) kind of unhappy. Maybe phabricator will be great, but I'll believe it when I see it. Please don't force future data and workflow into an as-yet unproven tool. Thanks. Ben ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Milan Sreckovic wrote: One thing that hasn't been explicitly mentioned, and I hope switching to phabricator would fix it (though it does sounds like an orthogonal issue) - the patches that are attached to bugzilla are often not the ones that actually landed, because last minute changes were made and landed manually. Will that get better with phabricator? the switch to phabricator won't directly address that issue. requiring autoland will. this is a topic that came up on this list when requiring landing through autoland was proposed - https://groups.google.com/d/msg/mozilla.dev.platform/hp5_6OAKV_c/JNWydrDVBAAJ it's a policy decision that hasn't been completely decided. how our team currently works when using mozreview is if there's an r+ with "fix on commit" changes required, we'd make those changes, push up the revised version to mozreview, carry forward the r+, and use autoland to land. -- glob — engineering productivity — moz://a ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Perfect, love it. One thing that hasn't been explicitly mentioned, and I hope switching to phabricator would fix it (though it does sounds like an orthogonal issue) - the patches that are attached to bugzilla are often not the ones that actually landed, because last minute changes were made and landed manually. Will that get better with phabricator? On 12-Jul-17 11:54, Byron Jones wrote: Consider that we are talking about "turning off" mozreview now. Will all the bugzilla links to those reviews go dead? Or do we have to maintain a second service in read-only mode forever? the patches will be archived in some form. how this looks is yet to be fully fleshed out - ideas currently include archiving and updating bugzilla to point to their new location (eg. s3), or uploading patches directly to bugzilla. -- - Milan (mi...@mozilla.com) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Consider that we are talking about "turning off" mozreview now. Will all the bugzilla links to those reviews go dead? Or do we have to maintain a second service in read-only mode forever? the patches will be archived in some form. how this looks is yet to be fully fleshed out - ideas currently include archiving and updating bugzilla to point to their new location (eg. s3), or uploading patches directly to bugzilla. -- glob — engineering productivity — moz://a ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 12-Jul-17 11:27, Byron Jones wrote: ... But indeed having also the patches in bugzilla would be good. no, it would be bad for patches to be duplicated into bugzilla. we're moving from bugzilla/mozreview to phabricator for code review, duplicating phabricate reviews back into the old systems seems like a step backwards (and i'm not even sure what the value is of doing so). I believe the answer to this depends on the answer to the "what happens to mozreview patches" after we switch to phabricator. If the answer was to be "you lose all of those", then the argument could be "we don't want to lose the patches once we switch from phabricator to product X in 20 years". -- - Milan (mi...@mozilla.com) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
Yeah, I live in the assumption that bugzilla bugs will contain all the review information also in phabricator era. i believe the current plan is to mirror just the outcome of the review to bugzilla (ie. that a review exists, and set the review flag). if comments should be mirrored to bugzilla has been a topic of much discussion, so this may change. i'm of the opinion that *not* mirroring comments to bugzilla is the right thing to do. phabricator provides a better experience for tracking reviews and notifications (eg. you can watch files/directories and be notified when reviews are posted that touch that code). one of my largest concerns is duplicating comments into bugzilla splits review dialog between two systems making it harder to follow conversations (unless you do bidirectional sync, which has its own set of problems). But indeed having also the patches in bugzilla would be good. no, it would be bad for patches to be duplicated into bugzilla. we're moving from bugzilla/mozreview to phabricator for code review, duplicating phabricate reviews back into the old systems seems like a step backwards (and i'm not even sure what the value is of doing so). -- glob — engineering productivity — moz://a ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Diff-viewing in splinter (was: Phabricator Update, July 2017)
> On Jul 11, 2017, at 22:24, Mike Hommeywrote: > > Splinter is still a nice UI to look at patches already attached to bugs. > Please don't disable it. > > Mike As a note, we currently support viewing GitHub Pull Requests in splinter as if they were patches example: https://bugzilla.mozilla.org/page.cgi?id=splinter.html=1331305=8884864 (this has been possible for almost a year, but there were various issues preventing it from working in the data center) I would probably follow the route that RedHat Bugzilla is taking and make use of the lovely https://diff2html.xyz/ library, which may provide a much better patch viewing experience. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 07/12/2017 04:20 PM, Ben Kelly wrote: On Tue, Jul 11, 2017 at 11:49 PM, Martin Thomsonwrote: On Wed, Jul 12, 2017 at 1:34 PM, Byron Jones wrote: instead of disabling splinter for phabricator backed products, we could make it a read-only patch viewer. Given the number of bugs that exist with patches attached, that would be greatly appreciated. I would also assume that attaching patches won't stop completely either. It would be nice if patch information was always contained within the bug. Scattering it across tools makes it harder to figure out happened later when you are researching why changes were made. Consider that we are talking about "turning off" mozreview now. Will all the bugzilla links to those reviews go dead? Or do we have to maintain a second service in read-only mode forever? This would not be a problem if the patch/review information was also stored in bugzilla instead of solely placed in a separate tool. Yeah, I live in the assumption that bugzilla bugs will contain all the review information also in phabricator era. But indeed having also the patches in bugzilla would be good. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Tue, Jul 11, 2017 at 11:49 PM, Martin Thomsonwrote: > On Wed, Jul 12, 2017 at 1:34 PM, Byron Jones wrote: > > instead of disabling splinter for phabricator backed products, we could > make > > it a read-only patch viewer. > > Given the number of bugs that exist with patches attached, that would > be greatly appreciated. I would also assume that attaching patches > won't stop completely either. > It would be nice if patch information was always contained within the bug. Scattering it across tools makes it harder to figure out happened later when you are researching why changes were made. Consider that we are talking about "turning off" mozreview now. Will all the bugzilla links to those reviews go dead? Or do we have to maintain a second service in read-only mode forever? This would not be a problem if the patch/review information was also stored in bugzilla instead of solely placed in a separate tool. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Wed, Jul 12, 2017 at 1:34 PM, Byron Joneswrote: > instead of disabling splinter for phabricator backed products, we could make > it a read-only patch viewer. Given the number of bugs that exist with patches attached, that would be greatly appreciated. I would also assume that attaching patches won't stop completely either. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
To answer the other part of your question, MozReview will be disabled for active use across the board, but it is currently used by a small number of projects. Splinter will be disabled on a per-product basis, as there may be some projects that can't, won't, or shouldn't be migrated to Phabricator. Splinter is still a nice UI to look at patches already attached to bugs. Please don't disable it. excellent point; thanks for that feedback. instead of disabling splinter for phabricator backed products, we could make it a read-only patch viewer. -- glob — engineering productivity — moz://a ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Tue, Jul 11, 2017 at 09:59:57PM -0400, Mark Côté wrote: > On 2017-07-11 9:51 PM, Martin Thomson wrote: > > On Wed, Jul 12, 2017 at 6:41 AM, Mark Côtéwrote: > > > * MozReview and Splinter turned off in early December. > > > > Is this bugzilla-wide? I know that other project use splinter still. > > Will those projects be able to use phabricator for their projects? > > > > For instance, NSS uses a separate instance of phabricator, but not all > > the developers are using it already. I don't think that we have NSPR > > setup yet. > > > > Yes, we welcome other Mozilla-related projects to use the new Phabricator > cluster. In fact, we're looking for projects outside of > Firefox/mozilla-central to be part of our pre-release period, which will > help us validate the Bugzilla integration. Migrating NSS over to the main > instance is one candidate, and NSPR sounds like a possibility too. > > To answer the other part of your question, MozReview will be disabled for > active use across the board, but it is currently used by a small number of > projects. Splinter will be disabled on a per-product basis, as there may be > some projects that can't, won't, or shouldn't be migrated to Phabricator. Splinter is still a nice UI to look at patches already attached to bugs. Please don't disable it. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On 2017-07-11 9:51 PM, Martin Thomson wrote: On Wed, Jul 12, 2017 at 6:41 AM, Mark Côtéwrote: * MozReview and Splinter turned off in early December. Is this bugzilla-wide? I know that other project use splinter still. Will those projects be able to use phabricator for their projects? > > For instance, NSS uses a separate instance of phabricator, but not all > the developers are using it already. I don't think that we have NSPR > setup yet. > Yes, we welcome other Mozilla-related projects to use the new Phabricator cluster. In fact, we're looking for projects outside of Firefox/mozilla-central to be part of our pre-release period, which will help us validate the Bugzilla integration. Migrating NSS over to the main instance is one candidate, and NSPR sounds like a possibility too. To answer the other part of your question, MozReview will be disabled for active use across the board, but it is currently used by a small number of projects. Splinter will be disabled on a per-product basis, as there may be some projects that can't, won't, or shouldn't be migrated to Phabricator. Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
On Wed, Jul 12, 2017 at 6:41 AM, Mark Côtéwrote: > * MozReview and Splinter turned off in early December. Is this bugzilla-wide? I know that other project use splinter still. Will those projects be able to use phabricator for their projects? For instance, NSS uses a separate instance of phabricator, but not all the developers are using it already. I don't think that we have NSPR setup yet. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
We're currently trying to figure that out. It's unlikely that it will be available for the initial launch of Phabricator, but we hope to have it not too long after. I'll have an update in a couple weeks. Mark On 2017-07-11 7:32 PM, Chris Pearce wrote: What is the status of push-to-review support? Chris. On Wednesday, July 12, 2017 at 8:42:06 AM UTC+12, Mark Côté wrote: Hi all, here's a brief update on the project to deploy and integrate Phabricator at Mozilla: * Development Phabricator instance is up at https://mozphab.dev.mozaws.net/, authenticated via bugzilla-dev.allizom.org. * Development, read-only UI for Lando (the new automatic-landing service) has been deployed. * Work is proceeding on matching viewing restrictions on Phabricator revisions (review requests) to associated confidential bugs. * Work is proceeding on the internals of Lando to land Phabricator revisions to the autoland Mercurial branch. * Pre-release of Phabricator, without Lando, targeted for mid-August. * General release of Phabricator and Lando targeted for late September or early October. * MozReview and Splinter turned off in early December. I have a blog post up with many more details: https://mrcote.info/blog/2017/07/11/phabricator-update/ More to come! Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Phabricator Update, July 2017
What is the status of push-to-review support? Chris. On Wednesday, July 12, 2017 at 8:42:06 AM UTC+12, Mark Côté wrote: > Hi all, here's a brief update on the project to deploy and integrate > Phabricator at Mozilla: > > * Development Phabricator instance is up at > https://mozphab.dev.mozaws.net/, authenticated via > bugzilla-dev.allizom.org. > > * Development, read-only UI for Lando (the new automatic-landing > service) has been deployed. > > * Work is proceeding on matching viewing restrictions on Phabricator > revisions (review requests) to associated confidential bugs. > > * Work is proceeding on the internals of Lando to land Phabricator > revisions to the autoland Mercurial branch. > > * Pre-release of Phabricator, without Lando, targeted for mid-August. > > * General release of Phabricator and Lando targeted for late September > or early October. > > * MozReview and Splinter turned off in early December. > > I have a blog post up with many more details: > https://mrcote.info/blog/2017/07/11/phabricator-update/ > > More to come! > > Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Phabricator Update, July 2017
Hi all, here's a brief update on the project to deploy and integrate Phabricator at Mozilla: * Development Phabricator instance is up at https://mozphab.dev.mozaws.net/, authenticated via bugzilla-dev.allizom.org. * Development, read-only UI for Lando (the new automatic-landing service) has been deployed. * Work is proceeding on matching viewing restrictions on Phabricator revisions (review requests) to associated confidential bugs. * Work is proceeding on the internals of Lando to land Phabricator revisions to the autoland Mercurial branch. * Pre-release of Phabricator, without Lando, targeted for mid-August. * General release of Phabricator and Lando targeted for late September or early October. * MozReview and Splinter turned off in early December. I have a blog post up with many more details: https://mrcote.info/blog/2017/07/11/phabricator-update/ More to come! Mark ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform