Re: Phabricator Update, July 2017

2017-07-24 Thread Mark Côté
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

2017-07-19 Thread Randell Jesup
>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

2017-07-19 Thread Randell Jesup
>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

2017-07-17 Thread Edmund Wong
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

2017-07-17 Thread Edmund Wong
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

2017-07-17 Thread Mark Côté

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

2017-07-17 Thread Ben Kelly
On Mon, Jul 17, 2017 at 12:27 PM, Gregory Szorc  wrote:

> 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

2017-07-17 Thread Edmund Wong
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

2017-07-17 Thread Mark Côté
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

2017-07-17 Thread Mike Hoye

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

2017-07-17 Thread Gregory Szorc


> On Jul 15, 2017, at 23:36, Gabriele Svelto  wrote:
> 
>> 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

2017-07-17 Thread jwood
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

2017-07-16 Thread Edmund Wong
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

2017-07-16 Thread Gabriele Svelto
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

2017-07-14 Thread Ben Kelly
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

2017-07-14 Thread Mike Hommey
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

2017-07-14 Thread Ben Kelly
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

2017-07-14 Thread Milan Sreckovic

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

2017-07-14 Thread Mark Côté

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

2017-07-14 Thread wax miguel


I'm 

> On 14 Jul 2017, at 11:39, 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...
> 
> -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

2017-07-14 Thread wax miguel
lets all try

I'm 

> On 14 Jul 2017, at 13:48, Jim Blandy  wrote:
> 
> 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

2017-07-14 Thread Boris Zbarsky

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

2017-07-13 Thread Jim Blandy
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


Re: Phabricator Update, July 2017

2017-07-13 Thread Jim Blandy
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 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...
>
> -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

2017-07-13 Thread Boris Zbarsky

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

2017-07-13 Thread Gregory Szorc
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

2017-07-13 Thread Mark Côté

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.


Mark
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Phabricator Update, July 2017

2017-07-13 Thread David Anderson
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

2017-07-13 Thread Joe Hildebrand
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

2017-07-13 Thread Randell Jesup
>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).

>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

2017-07-13 Thread Randell Jesup
>>> 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

2017-07-12 Thread Botond Ballo
On Wed, Jul 12, 2017 at 11:54 AM, 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.

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

2017-07-12 Thread Boris Zbarsky

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

2017-07-12 Thread Daniel Veditz
On Wed, Jul 12, 2017 at 8:54 AM, 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.
>

​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

2017-07-12 Thread Ben Kelly
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.

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

2017-07-12 Thread Byron Jones

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

2017-07-12 Thread Milan Sreckovic

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

2017-07-12 Thread Byron Jones



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

2017-07-12 Thread Milan Sreckovic

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

2017-07-12 Thread Byron Jones


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)

2017-07-12 Thread Dylan Hardison

> On Jul 11, 2017, at 22:24, Mike Hommey  wrote:
> 
> 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

2017-07-12 Thread smaug

On 07/12/2017 04:20 PM, Ben Kelly wrote:

On Tue, Jul 11, 2017 at 11:49 PM, Martin Thomson  wrote:


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

2017-07-12 Thread Ben Kelly
On Tue, Jul 11, 2017 at 11:49 PM, Martin Thomson  wrote:

> 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

2017-07-11 Thread Martin Thomson
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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Phabricator Update, July 2017

2017-07-11 Thread Byron Jones

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

2017-07-11 Thread Mike Hommey
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

2017-07-11 Thread Mark Côté

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

2017-07-11 Thread Martin Thomson
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

2017-07-11 Thread Mark Côté
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

2017-07-11 Thread Chris Pearce
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

2017-07-11 Thread Mark Côté
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