Re: The future of commit access policy for core Firefox

[Nicholas Nethercote]

On Sat, Mar 11, 2017 at 2:23 PM, smaug via governance <
governa...@lists.mozilla.org> wrote:
>
> I'd be ok to do a quick r+ if interdiff was working well.

Depending on the relative timezones of the reviewer and reviewee, that
could delay landing by 24 hours or even a whole weekend.

In general there seems to be a large amount of support in this thread for
continuing to allow the r+-with-minor-fixes option.

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

Re: The future of commit access policy for core Firefox

[L. David Baron]

On Friday 2017-03-10 19:33 -0800, Eric Rescorla wrote:
> We have been using Phabricator for our reviews in NSS and its interdiffs
> work pretty well
> (modulo rebases, which are not so great), and it's very easy to handle LGTM
> with
> nits and verify the nits.

For what it's worth, I think proper interdiffs have two columns of
[ -+] at the beginning of each line, not one column like diffs do.
I've gotten used to reading interdiffs as diff -u of a diff -u, and
while it takes a little getting used to, but once you're used to it,
it actually represents what an interdiff is and is quite usable.  I
think anything that pretends that something like this:

  // Frame has a LayerActivityProperty property
  FRAME_STATE_BIT(Generic, 54, NS_FRAME_HAS_LAYER_ACTIVITY_PROPERTY)
  
+ // Frame owns anonymous boxes whose style contexts it will need to update 
during
+ // a stylo tree traversal.
+ FRAME_STATE_BIT(Generic, 55, NS_FRAME_OWNS_ANON_BOXES)
+ 
 +// If this bit is set, then reflow may be dispatched from the current
 +// frame instead of the root frame.
-+FRAME_STATE_BIT(Generic, 55, NS_FRAME_DYNAMIC_REFLOW_ROOT)
++FRAME_STATE_BIT(Generic, 56, NS_FRAME_DYNAMIC_REFLOW_ROOT)
 +
  // Set for all descendants of MathML sub/supscript elements (other than the
  // base frame) to indicate that the SSTY font feature should be used.
  FRAME_STATE_BIT(Generic, 58, NS_FRAME_MATHML_SCRIPT_DESCENDANT)

can be represented with only one column of [+- ] at the beginning
is going to fail for some substantial set of important cases (such
as rebases, as you mention).

(That's a piece of interdiff from rebasing my own patch queue
earlier this week.)

-David

-- 
턞   L. David Baron http://dbaron.org/   턂
턢   Mozilla  https://www.mozilla.org/   턂
 Before I built a wall I'd ask to know
 What I was walling in or walling out,
 And to whom I was like to give offense.
   - Robert Frost, Mending Wall (1914)


signature.asc
Description: PGP signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Heads Up: /storage upgraded from version 1.0 to 2.0

[rnewman]

> As an example of why "backup the db" is harder than it sounds, you would
> need to backup the entire storage subsystem. 

Mossop suggested I chime in on this thread, but I think Ben has covered much of 
what I'd say!

Some set of schema changes is safe or possible to downgrade: rearranging 
deckchairs, reversible optimizations, etc.

Some additional set can be downgraded thanks to semantic restrictions in place: 
adding a purely optional field, for example. (That doesn't mean that it's 
trivial to make SQLite or another disk format behave correctly, of course, and 
one can't always tell when an optional field isn't really optional in every 
circumstance….)

A broader set is very difficult to downgrade. Code to get the downgrade right 
is hard to fix in the wild and hard to test. Systems that spread data across 
multiple data sources (multiple files, prefs, caches…) get really hairy.

Leaving old files on disk is almost never the correct solution: think of the 
effect that this has on Sync, for example. If you're thinking about leaving 
files on disk for anything other than disaster recovery, you should reconsider.

In short: it's hard to make non-trivial changes to a schema, particularly in 
SQLite, and expect to be able to hop between versions. The tradeoff in effort 
and bugs versus benefit is slanted. If we're communicating to our users that 
they can safely do so — "just try this out on Nightly with your main profile!" 
— we should stop doing so.

Two alternatives to consider:

- Support two storage versions in parallel for a while: ship the code and use 
the old one until the switch is flipped. This is acceptable if you're not in a 
rush to ship. Hands up if you're not in a rush to ship.
- Don't allow downgrades.

The difficulty of evolving storage was one of the motivations for Project 
Mentat. Schema fragments in Mentat can evolve additively without a version bump 
if the additions are safe. Clients can introspect the schema and operate in a 
degraded mode if they can get what they need. Additive fragments don't affect 
older fragments at all. And a higher-level schema makes some of the DB churn we 
see in our Firefoxes less likely.

Storage is hard.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: The future of commit access policy for core Firefox

[Eric Rescorla]

On Fri, Mar 10, 2017 at 7:23 PM, smaug via governance <
governa...@lists.mozilla.org> wrote:

> On 03/10/2017 12:59 AM, Bobby Holley wrote:
>
>> At a high level, I think the goals here are good.
>>
>> However, the tooling here needs to be top-notch for this to work, and the
>> standard approach of flipping on an MVP and dealing with the rest in
>> followup bugs isn't going to be acceptable for something so critical to
>> our
>> productivity as landing code. The only reason more developers aren't
>> complaining about the MozReview+autoland workflow right now is that it's
>> optional.
>>
>> The busiest and most-productive developers (ehsan, bz, dbaron, etc) tend
>> not to pay attention to new workflows because they have too much else on
>> their plate. The onus needs to be on the team deploying this to have the
>> highest-volume committers using the new system happily and voluntarily
>> before it becomes mandatory. That probably means that the team should not
>> have any deadline-oriented incentives to ship it before it's ready.
>>
>> Also, getting rid of "r+ with comments" is a non-starter.
>>
>
> FWIW, with my reviewer hat on, I think that is not true, _assuming_ there
> is a reliable interdiff for patches.
> And not only MozReview patches but for all the patches. (and MozReview
> interdiff isn't reliable, it has dataloss issues so it
> doesn't really count there.).
> I'd be ok to do a quick r+ if interdiff was working well.
>

Without taking a position on the broader proposal, I agree with this.

We have been using Phabricator for our reviews in NSS and its interdiffs
work pretty well
(modulo rebases, which are not so great), and it's very easy to handle LGTM
with
nits and verify the nits.

-Ekr


>
>
>
> -Olli
>
>
>> bholley
>>
>>
>> On Thu, Mar 9, 2017 at 1:53 PM, Mike Connor  wrote:
>>
>> (please direct followups to dev-planning, cross-posting to governance,
>>> firefox-dev, dev-platform)
>>>
>>>
>>> Nearly 19 years after the creation of the Mozilla Project, commit access
>>> remains essentially the same as it has always been.  We've evolved the
>>> vouching process a number of times, CVS has long since been replaced by
>>> Mercurial & others, and we've taken some positive steps in terms of
>>> securing the commit process.  And yet we've never touched the core idea
>>> of
>>> granting developers direct commit access to our most important
>>> repositories.  After a large number of discussions since taking ownership
>>> over commit policy, I believe it is time for Mozilla to change that
>>> practice.
>>>
>>> Before I get into the meat of the current proposal, I would like to
>>> outline
>>> a set of key goals for any change we make.  These goals have been
>>> informed
>>> by a set of stakeholders from across the project including the
>>> engineering,
>>> security, release and QA teams.  It's inevitable that any significant
>>> change will disrupt longstanding workflows.  As a result, it is critical
>>> that we are all aligned on the goals of the change.
>>>
>>>
>>> I've identified the following goals as critical for a responsible commit
>>> access policy:
>>>
>>>
>>> - Compromising a single individual's credentials must not be
>>> sufficient
>>> to land malicious code into our products.
>>> - Two-factor auth must be a requirement for all users approving or
>>> pushing a change.
>>> - The change that gets pushed must be the same change that was
>>> approved.
>>> - Broken commits must be rejected automatically as a part of the
>>> commit
>>> process.
>>>
>>>
>>> In order to achieve these goals, I propose that we commit to making the
>>> following changes to all Firefox product repositories:
>>>
>>>
>>> - Direct commit access to repositories will be strictly limited to
>>> sheriffs and a subset of release engineering.
>>>- Any direct commits by these individuals will be limited to
>>> fixing
>>>bustage that automation misses and handling branch merges.
>>> - All other changes will go through an autoland-based workflow.
>>>- Developers commit to a staging repository, with scripting that
>>>connects the changeset to a Bugzilla attachment, and integrates
>>> with review
>>>flags.
>>>- Reviewers and any other approvers interact with the changeset as
>>>today (including ReviewBoard if preferred), with Bugzilla flags as
>>> the
>>>canonical source of truth.
>>>- Upon approval, the changeset will be pushed into autoland.
>>>- If the push is successful, the change is merged to
>>> mozilla-central,
>>>and the bug updated.
>>>
>>> I know this is a major change in practice from how we currently operate,
>>> and my ask is that we work together to understand the impact and
>>> concerns.
>>> If you find yourself disagreeing with the goals, let's have that
>>> discussion
>>> instead of arguing about the solution.  If you agree with the goals, but
>>> not the solution, I'd 

Re: The future of commit access policy for core Firefox

[Frank-Rainer Grahl]

Ehsan Akhgari wrote:

Even with a single reviewer, I often times end up making some trivial
changes to my patches to fix stupid mistakes and issues that I know the
reviewer doesn't care enough to want to look at before landing.  In
general our code review process has a lot of flexibility built into it,
and reviewers generally understand that the goal ultimately is to ensure
the quality of the produced code, so depending on the circumstances as a
reviewer I can treat a patch on different levels of scrutiny, from
anywhere between checking the actual landed patch and complaining if
something wasn't done in the way I asked to r+ing asking for a lot of
changes and trusting the author will do the right thing without needing
me look over their work more.

...

Same here. Automation is fine if everything goes according to plan but pushing 
manually is much less time consuming if something goes wrong e.g. a patch 
needs trivial changes to un-bitrot it. So there should still be a way to just 
push manually if needed or desired.


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

New requirement: New processes need AWSY support before landing

[Brad Lassey]

Hi all,
We seem to be taking advantage of our multi-process capabilities more and more 
these days. In order to fully understand the memory impact of these changes, 
any new process type needs to be fully supported by AWSY (Are We Slim Yet) 
prior to landing. If you have any questions, please have a look at the wiki 
[https://developer.mozilla.org/en-US/docs/Mozilla/Performance/AWSY] or contact 
Eric Rahm.

I should also note that we plan to start running AWSY as part of regular test 
automation (try and integration) soon. That work is being tracked by bug 
1272113.

Thanks,
Brad
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Sheriff Highlights and Summary in February 2017

[Kris Maglione]

On Fri, Mar 10, 2017 at 01:55:40PM +, David Burns wrote:

I went back and did some checks with autoland to servo and the results are
negligible. So from 01 February 2017 to 10 March 2017 (as of sending this
email). I have removed merge commits from the numbers.

Autoland:
Total Servo Sync Pushes: 152
Total Pushes: 1823
Total Backouts: 144
Percentage of backouts: 7.8990674712
Percentage of backouts without Servo: 8.61759425494

Mozilla-Inbound:
Total Pushes: 1472
Total Backouts: 166
Percentage of backouts: 11.277173913


Is there any way you can get these numbers in terms of patches, rather than 
pushes? Or, ideally, in terms of bugs landed and backed out? Pushes to inbound 
still often have patches for more than one bug, so if 4 bugs bets pushed to 
inbound in one push, and 4 land on autoland as separate pushes, and one gets 
backed out from each branch, the comparison isn't very useful.

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

Re: Sheriff Highlights and Summary in February 2017

[Chris Peterson]

I seem to recall gps saying, a few years ago, that the cost per push or 
Try run was about $36. It would be good to know the current cost so 
developers can feel more comfortable spending Mozilla money on 
"unnecessary" Try runs before landing.



On 3/10/2017 6:47 AM, Andrew Halberstadt wrote:

I don't have any data to back this up, but my suspicion is that a large
percentage
of backouts had try runs, but said try runs didn't run the jobs that failed
and caused
the backout. Imo, these kinds of backouts are (more) acceptable because it
means
the developer was trying to avoid doing a full try run, a practice that
should be
cheaper overall in the long run (and if done properly).

For example, you could either:
A) Do a full try run then push, almost guaranteeing you won't be backed
out. But
then you run every job twice and take longer to complete your bug, a
significant
cost.

B) Do a partial try run, running X% of the jobs yielding a Y% chance of
being
backed out.

There's some sweet spot between no try run, and try: -all which is the most
cost
effective (both in terms of AWS bill and developer time).

That being said, I think this is an issue of tools rather than policy.
Things like being
smarter about running jobs based on files changed and improving interfaces
to
selecting jobs on try, should help with backout rates. But the single
biggest thing we
could do is getting rid of integration branches altogether (and instead get
autoland to
merge directly from try). In this world, backouts would hardly even exist
anymore.

I believe we're already headed in this direction, albeit slowly.

-Andrew

On Fri, Mar 10, 2017 at 8:55 AM, David Burns  wrote:


I went back and did some checks with autoland to servo and the results are
negligible. So from 01 February 2017 to 10 March 2017 (as of sending this
email). I have removed merge commits from the numbers.

Autoland:
Total Servo Sync Pushes: 152
Total Pushes: 1823
Total Backouts: 144
Percentage of backouts: 7.8990674712
Percentage of backouts without Servo: 8.61759425494

Mozilla-Inbound:
Total Pushes: 1472
Total Backouts: 166
Percentage of backouts: 11.277173913


I will look into why, with more pushes, is resulting in fewer backouts. The
thing to note is that autoland, by its nature, does not allow us to fail
forward like inbound without having to get a sheriff to land the code.

I think, and this is my next area to investigate, is the 1 bug per push
(the autoland model) could be helping with the percentage of backouts being
lower.

David

On 7 March 2017 at 21:29, Chris Peterson  wrote:


On 3/7/2017 3:38 AM, Joel Maher wrote:


One large difference I see between autoland and mozilla-inbound is that

on

autoland we have many single commits/push whereas mozilla-inbound it is
fewer.  I see the Futurama data showing pushes and the sheriff report
showing total commits.



autoland also includes servo commits imported from GitHub that won't

break

Gecko. (They might break the linux64-stylo builds, but they won't be

backed

out for that.)

___
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: What are your use cases for the Touch Bar on the new MacBook Pro?

[Stephen A Pohl]

Thank you to everyone who has shared ideas so far. I've compiled a
project plan for this feature here:

https://docs.google.com/document/u/0/d/1hX5IVeFRdLyBCcMu9F_6rQi4wdmLv46UWmJ5muRpwcw/pub

tl;dr: The proposed path forward is to build a WebExtensions API that
can be used by both system addons and third-party addons. This allows
for individual teams to develop their own WebExtensions for specific use
cases without requiring dedicated widget:cocoa work.

Additional input and comments always welcome.

-spohl

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

Re: The future of commit access policy for core Firefox

[Frank-Rainer Grahl]

Me too. Have a phone which does what I need (calling people and be called:) ) 
No need for a mobile device which has its own security and usability problems.


FRG

Masatoshi Kimura wrote:

On 2017/03/10 6:53, Mike Connor wrote:

- Two-factor auth must be a requirement for all users approving or
pushing a change.


I have no mobile devices. How can I use 2FA?

Previously I was suggested to buy a new PC and SSD only to shorten the
build time. Now do I have to buy a smartphone only to contribute
Mozilla? What's the next?




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

Nightly updates frozen

[Ryan VanderMeulen]

Due to some font-related topcrashes, nightly build updates are currently
frozen on yesterday's build. Backouts that'll hopefully fix it are on
mozilla-central and respins are in progress. Once they're finished, RelEng
will un-freeze.

Thanks,
Ryan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: The future of commit access policy for core Firefox

[jwood]

On Friday, March 10, 2017 at 7:38:57 AM UTC-5, Masatoshi Kimura wrote:
> On 2017/03/10 6:53, Mike Connor wrote:
> >- Two-factor auth must be a requirement for all users approving or
> >pushing a change.
> 
> I have no mobile devices. How can I use 2FA?
> 
> Previously I was suggested to buy a new PC and SSD only to shorten the
> build time. Now do I have to buy a smartphone only to contribute
> Mozilla? What's the next?

Fwiw, Yubikey's are generally suitable 2FA devices, and are significantly 
cheaper than smartphones. I'd not be surprised if key contributors who need 
commit rights were able to apply to mozilla in order to acquire a yubikey 
should they be unable to procure on themselves. (I'm not a policy maker, so do 
NOT take my statement as any sort of promise)

~Justin Wood (Callek)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Sheriff Highlights and Summary in February 2017

[Andrew Halberstadt]

I don't have any data to back this up, but my suspicion is that a large
percentage
of backouts had try runs, but said try runs didn't run the jobs that failed
and caused
the backout. Imo, these kinds of backouts are (more) acceptable because it
means
the developer was trying to avoid doing a full try run, a practice that
should be
cheaper overall in the long run (and if done properly).

For example, you could either:
A) Do a full try run then push, almost guaranteeing you won't be backed
out. But
then you run every job twice and take longer to complete your bug, a
significant
cost.

B) Do a partial try run, running X% of the jobs yielding a Y% chance of
being
backed out.

There's some sweet spot between no try run, and try: -all which is the most
cost
effective (both in terms of AWS bill and developer time).

That being said, I think this is an issue of tools rather than policy.
Things like being
smarter about running jobs based on files changed and improving interfaces
to
selecting jobs on try, should help with backout rates. But the single
biggest thing we
could do is getting rid of integration branches altogether (and instead get
autoland to
merge directly from try). In this world, backouts would hardly even exist
anymore.

I believe we're already headed in this direction, albeit slowly.

-Andrew

On Fri, Mar 10, 2017 at 8:55 AM, David Burns  wrote:

> I went back and did some checks with autoland to servo and the results are
> negligible. So from 01 February 2017 to 10 March 2017 (as of sending this
> email). I have removed merge commits from the numbers.
>
> Autoland:
> Total Servo Sync Pushes: 152
> Total Pushes: 1823
> Total Backouts: 144
> Percentage of backouts: 7.8990674712
> Percentage of backouts without Servo: 8.61759425494
>
> Mozilla-Inbound:
> Total Pushes: 1472
> Total Backouts: 166
> Percentage of backouts: 11.277173913
>
>
> I will look into why, with more pushes, is resulting in fewer backouts. The
> thing to note is that autoland, by its nature, does not allow us to fail
> forward like inbound without having to get a sheriff to land the code.
>
> I think, and this is my next area to investigate, is the 1 bug per push
> (the autoland model) could be helping with the percentage of backouts being
> lower.
>
> David
>
> On 7 March 2017 at 21:29, Chris Peterson  wrote:
>
> > On 3/7/2017 3:38 AM, Joel Maher wrote:
> >
> >> One large difference I see between autoland and mozilla-inbound is that
> on
> >> autoland we have many single commits/push whereas mozilla-inbound it is
> >> fewer.  I see the Futurama data showing pushes and the sheriff report
> >> showing total commits.
> >>
> >
> > autoland also includes servo commits imported from GitHub that won't
> break
> > Gecko. (They might break the linux64-stylo builds, but they won't be
> backed
> > out for that.)
> >
> > ___
> > 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: Sheriff Highlights and Summary in February 2017

[David Burns]

I went back and did some checks with autoland to servo and the results are
negligible. So from 01 February 2017 to 10 March 2017 (as of sending this
email). I have removed merge commits from the numbers.

Autoland:
Total Servo Sync Pushes: 152
Total Pushes: 1823
Total Backouts: 144
Percentage of backouts: 7.8990674712
Percentage of backouts without Servo: 8.61759425494

Mozilla-Inbound:
Total Pushes: 1472
Total Backouts: 166
Percentage of backouts: 11.277173913


I will look into why, with more pushes, is resulting in fewer backouts. The
thing to note is that autoland, by its nature, does not allow us to fail
forward like inbound without having to get a sheriff to land the code.

I think, and this is my next area to investigate, is the 1 bug per push
(the autoland model) could be helping with the percentage of backouts being
lower.

David

On 7 March 2017 at 21:29, Chris Peterson  wrote:

> On 3/7/2017 3:38 AM, Joel Maher wrote:
>
>> One large difference I see between autoland and mozilla-inbound is that on
>> autoland we have many single commits/push whereas mozilla-inbound it is
>> fewer.  I see the Futurama data showing pushes and the sheriff report
>> showing total commits.
>>
>
> autoland also includes servo commits imported from GitHub that won't break
> Gecko. (They might break the linux64-stylo builds, but they won't be backed
> out for that.)
>
> ___
> 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: The future of commit access policy for core Firefox

[Masatoshi Kimura]

On 2017/03/10 14:00, Mike Hommey wrote:
> While we're talking about drag on productivity, there's already one that
> comes from autoland already, which is that you can't easily land fixups
> for stupid mistakes (like, a build failure on one platform, or other
> lame things that happen when things land).
> 
> You either have to get a sheriff to land the fixup for you on autoland,
> or to back you out, in which case you're back to square one and need to
> reland the whole thing.
> 
> If on top of that, you also need another round of reviews...

Yes, it is really frustrating and wasting bandwidth of both developers
and shriffs. It is one of reasons I dislike mozreview/autoland.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: The future of commit access policy for core Firefox

[Masatoshi Kimura]

On 2017/03/10 6:53, Mike Connor wrote:
>- Two-factor auth must be a requirement for all users approving or
>pushing a change.

I have no mobile devices. How can I use 2FA?

Previously I was suggested to buy a new PC and SSD only to shorten the
build time. Now do I have to buy a smartphone only to contribute
Mozilla? What's the next?

-- 
vyv03...@nifty.ne.jp
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Tracking bug for removals after XPCOM extensions are no more?

[Henri Sivonen]

Do we have a tracking bug for all the stuff that we can and should
remove once we no longer support XPCOM extensions?

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Please write good commit messages before asking for code review

[Honza Bambas]

Thanks for bringing this up here, Ehsan.

I have to concur.  For more then just trivial patches I usually ask for 
a description of the patch in bugzilla - the commit message is usually 
not enough.  Anything that can help the review by understanding the 
intention and structure of the patch helps a lot and often is even 
necessary.  The more the better.  From time to time I r- a patch and ask 
for this info if I find it too complicated to review without it.


Thank you as well!

-hb-

On 3/9/17 8:46 PM, Ehsan Akhgari wrote:

I review a large number of patches on a typical day, and usually I have to
spend a fair amount of time to just understand what the patch is doing.  As
the patch author, you can do a lot to help make this easier by *writing
better commit messages*.  Starting now, I'm going to try out a new practice
for a while: I'm going to first review the commit message of all patches,
and if I can't understand what the patch does by reading the commit message
before reading any of the code, I'll r- and ask for another version of the
patch.

I thank you in advance for your cooperation by writing great commit
messages.  Happy hacking!

Cheers,



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

Re: Please write good commit messages before asking for code review

[Gabriele Svelto]

On 09/03/2017 22:35, Eric Rescorla wrote:
> I'm in favor of good commit messages, but I would note that current m-c
> convention really pushes against this, because people seem to feel that
> commit messages should be one line. Not sure what to do about that,
> but thought I would mention it.

Yeah, I always thought we had a policy of one-line commit messages but
I think that originated back in the day when one would attach a patch on
Bugzilla and added a comment there describing it in detail. With
mozreview it seems more sensible to put that detailed description in the
commit message.

 Gabriele




signature.asc
Description: OpenPGP digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform