Re: SpiderMonkey and XPConnect style changing from |T *p| to |T* p|
On 03/27/2015 12:19 AM, Bobby Holley wrote: Can we switch from 4-space to 2-space indentation at some point too? Yeah good idea, and we should probably scratch everything we have done so far and rewrite everything in whitespace. -- Nicolas B. Pierron ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: SpiderMonkey and XPConnect style changing from |T *p| to |T* p|
On Thu, Mar 26, 2015 at 10:37 PM, Birunthan Mohanathas < birunt...@mohanathas.com> wrote: > On 27 March 2015 at 01:19, Bobby Holley wrote: > > Can we switch from 4-space to 2-space indentation at some point too? > > Together, those would remove the most glaring formatting differences. The > > others (bracing single-line consequents, aNamingWarts, etc) might be > harder. > > I've done all of that and more on a few directories (e.g. xpcom/ in > bug 1046841). If we reach consensus regarding js/ style, I could take > care of the conversion over the summer. > That would be awesome! I'm in favor, and I'm pretty sure that jorendorff and jandem would be as well. Jason/Jan, can you confirm? On Fri, Mar 27, 2015 at 4:16 AM, Nicolas B. Pierron < nicolas.b.pier...@mozilla.com> wrote: On 03/27/2015 12:19 AM, Bobby Holley wrote: > Can we switch from 4-space to 2-space indentation at some point too? > Yeah good idea, and we should probably scratch everything we have done so far and rewrite everything in whitespace. I don't follow the point you're trying to make. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: SpiderMonkey and XPConnect style changing from |T *p| to |T* p|
On Fri, Mar 27, 2015 at 5:12 PM, Bobby Holley wrote: > On Thu, Mar 26, 2015 at 10:37 PM, Birunthan Mohanathas < > birunt...@mohanathas.com> wrote: > >> I've done all of that and more on a few directories (e.g. xpcom/ in >> bug 1046841). If we reach consensus regarding js/ style, I could take >> care of the conversion over the summer. >> > > That would be awesome! I'm in favor, and I'm pretty sure that jorendorff > and jandem would be as well. Jason/Jan, can you confirm? > I don't have strong opinions on coding style, as long as we pick one and use it consistently. MFBT is a good argument to switch: SpiderMonkey uses Vector and some others in a ton of places and if eventually all MFBT classes will have methods that start with an uppercase letter, it will get a bit awkward. On the other hand, going from 99 columns to 80 will also be annoying in a bunch of places. It's up to Jason :) Jan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
From nsIntSize to gfx::IntSize
As many of you know, the introduction of Moz2D a while ago added new size, point and rect classes which are equivalent* to the ones that already existed in tree (nsIntSize, etc.). Juggling back and forth between the Moz2D classes and their thebes equivalent is pretty annoying and until now we have been progressively replacing uses of thebes classes by Moz2D ones (a slow process bearing the exquisite name "Moz2Dification"). Moz2Dification of size classes hasn't been very efficient, so I decided to just remove the integer thebes size/rect/point classes and make then typedefs of the moz2D ones. I will soon (probably over the weekend) land a patch set that does this for nsIntSize/gfx::IntSize. What this means if you are writing code outside of the gfx/ directory: Not much. Apologies if my patch queue causes some merge conflicts. It's up to module owners to decide if they still want refer to integer sizes as nsIntSize (the typedef) or as gfx::IntSize (the real thing!). You won't need to use the conversion helpers ToIntSize and ThebesIntSize anymore, since nsIntSize and gfx::IntSize will be one and the same, and by the way those conversion helpers are being removed which is most likely the reason if you run into merge conflicts. If you write code under the gfx/ directory, please use gfx::IntSize. After this lands there won't be any reason to refer to it as nsIntSize, so let's make things consistent. nsIntRect and nsIntPoint will soon share the same fate. Cheers, Nical * This only true for integer classes (IntSize, ect.) because the non-integer ones use double precision floats in thebes and regular floats in Moz2D, so we can't just make the switch in one go as easily. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Updating the policy for Talos performance regression in 2015
Old thread but now that we're 3 months into 2015, has this new policy been effective at getting perf regressions fixed or at least deliberately accepted? Lawrence On Fri, Dec 19, 2014 at 1:33 PM, wrote: > Great questions folks. > > :bsmedberg has answered the questions quite well, let me elaborate: > Before a bug can be marked as resolved:fixed we need to verify the > regression is actually fixed. In many cases we will fix a large portion of > the regression and accept the small remainder. > > We do keep track of all the bugs filed per version (firefox 36 example: > https://bugzilla.mozilla.org/show_bug.cgi?id=1084461) > > these get looked at more specifically during each uplift. > > I will update the verbage next week to call out how these will be followed > up and posted to: > https://www.mozilla.org/hacking/regression-policy.html > > Do speak up if this should be posted elsewhere or linked from a specific > location. > ___ > 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: Updating the policy for Talos performance regression in 2015
As one of the primary people sheriffing alerts, I have found that we get decisions made much faster as a result of this policy. I would be interested to hear if others have differing opinions as I could be seeing this with tunnel vision. -Joel On Fri, Mar 27, 2015 at 4:34 PM, Lawrence Mandel wrote: > Old thread but now that we're 3 months into 2015, has this new policy been > effective at getting perf regressions fixed or at least deliberately > accepted? > > Lawrence > > On Fri, Dec 19, 2014 at 1:33 PM, wrote: > >> Great questions folks. >> >> :bsmedberg has answered the questions quite well, let me elaborate: >> Before a bug can be marked as resolved:fixed we need to verify the >> regression is actually fixed. In many cases we will fix a large portion of >> the regression and accept the small remainder. >> >> We do keep track of all the bugs filed per version (firefox 36 example: >> https://bugzilla.mozilla.org/show_bug.cgi?id=1084461) >> >> these get looked at more specifically during each uplift. >> >> I will update the verbage next week to call out how these will be >> followed up and posted to: >> https://www.mozilla.org/hacking/regression-policy.html >> >> Do speak up if this should be posted elsewhere or linked from a specific >> location. >> ___ >> 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: SpiderMonkey and XPConnect style changing from |T *p| to |T* p|
On 03/27/2015 04:42 PM, Jan De Mooij wrote: On the other hand, going from 99 columns to 80 will also be annoying in a bunch of places. I don't think that's necessary. Let's relax our 80-column rule in Gecko instead. We can keep it as a recommendation, but allow longer lines when it improves readability of the code. In Layout, where I spend most of my time, we tend to prefer ReallyLongAndDescriptiveNames which often leads to weird line- breaking and indentation to make the code fit in 80 columns. This makes the code hard to read and maintain, not to mention the *significant* waste of time for the writer to try to squeeze it down to fit. I have to admit that I have sometimes chosen shorter and less descriptive names just to avoid the problem and I've seen others do the same (most recently today actually when reviewing some code). So let's change the project-wide coding rules instead to allow 99 columns as the hard limit, but keep 80 columns as the recommended (soft) limit. /Mats ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: From nsIntSize to gfx::IntSize
Sounds good. But, is gfx::IntSize going to get a "ToAppUnits" method like nsIntSize has? As a followup it's probably worth replacing all of nsIntSize with gfx::IntSize. Rob -- oIo otoeololo oyooouo otohoaoto oaonoyooonoeo owohooo oioso oaonogoroyo owoiotoho oao oboroootohoeoro oooro osoiosotoeoro owoiololo oboeo osouobojoeocoto otooo ojouodogomoeonoto.o oAogoaoiono,o oaonoyooonoeo owohooo osoaoyoso otooo oao oboroootohoeoro oooro osoiosotoeoro,o o‘oRoaocoao,o’o oioso oaonosowoeoroaoboloeo otooo otohoeo ocooouoroto.o oAonodo oaonoyooonoeo owohooo osoaoyoso,o o‘oYooouo ofolo!o’o owoiololo oboeo oiono odoaonogoeoro ooofo otohoeo ofoioroeo ooofo ohoeololo. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: From nsIntSize to gfx::IntSize
Probably safe for the integer types, but can we add strong assertions when converting from Thebes and Moz2D floats? Bugs like this one are tough to debug: https://bugzilla.mozilla.org/show_bug.cgi?id=1091709 Thanks! --Jet On Fri, Mar 27, 2015 at 9:44 AM, Nicolas Silva wrote: > As many of you know, the introduction of Moz2D a while ago added new size, > point and rect classes which are equivalent* to the ones that already > existed in tree (nsIntSize, etc.). > > Juggling back and forth between the Moz2D classes and their thebes > equivalent is pretty annoying and until now we have been progressively > replacing uses of thebes classes by Moz2D ones (a slow process bearing the > exquisite name "Moz2Dification"). Moz2Dification of size classes hasn't > been very efficient, so I decided to just remove the integer thebes > size/rect/point classes and make then typedefs of the moz2D ones. > > I will soon (probably over the weekend) land a patch set that does this for > nsIntSize/gfx::IntSize. > > What this means if you are writing code outside of the gfx/ directory: Not > much. Apologies if my patch queue causes some merge conflicts. > It's up to module owners to decide if they still want refer to integer > sizes as nsIntSize (the typedef) or as gfx::IntSize (the real thing!). You > won't need to use the conversion helpers ToIntSize and ThebesIntSize > anymore, since nsIntSize and gfx::IntSize will be one and the same, and by > the way those conversion helpers are being removed which is most likely the > reason if you run into merge conflicts. > > If you write code under the gfx/ directory, please use gfx::IntSize. After > this lands there won't be any reason to refer to it as nsIntSize, so let's > make things consistent. > > nsIntRect and nsIntPoint will soon share the same fate. > > Cheers, > > Nical > > * This only true for integer classes (IntSize, ect.) because the > non-integer ones use double precision floats in thebes and regular floats > in Moz2D, so we can't just make the switch in one go as easily. > ___ > 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: SpiderMonkey and XPConnect style changing from |T *p| to |T* p|
On Fri, Mar 27, 2015 at 2:04 PM, Mats Palmgren wrote: So let's change the project-wide coding rules instead to allow 99 > columns as the hard limit, but keep 80 columns as the recommended > (soft) limit. > I think we should avoid opening up a can of worms on the merits of different styles, and instead focus on the most pragmatic ways to unify Gecko and JS style. Under that framework, Mats' proposal makes a lot of sense. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: SpiderMonkey and XPConnect style changing from |T *p| to |T* p|
On 03/27/2015 11:51 PM, Bobby Holley wrote: On Fri, Mar 27, 2015 at 2:04 PM, Mats Palmgren wrote: So let's change the project-wide coding rules instead to allow 99 columns as the hard limit, but keep 80 columns as the recommended (soft) limit. I think we should avoid opening up a can of worms on the merits of different styles, and instead focus on the most pragmatic ways to unify Gecko and JS style. Under that framework, Mats' proposal makes a lot of sense. I do not see the advantages of having huge patches to rewrite an entire project just for the benefit of having only one style guide. What I see with such patches, is pain to rebase patches, pain to change habs of the developers, and security issues as contributors (including employees) do not look for the original authors. From my point of view, the only time where such patches sounds acceptable, is when you are trying to take over a dead project, and as far as I know SpiderMonkey is far from being a dead project. -- Nicolas B. Pierron ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: SpiderMonkey and XPConnect style changing from |T *p| to |T* p|
On Fri, Mar 27, 2015 at 5:32 PM, Nicolas B. Pierron < nicolas.b.pier...@mozilla.com> wrote: > On 03/27/2015 11:51 PM, Bobby Holley wrote: > >> On Fri, Mar 27, 2015 at 2:04 PM, Mats Palmgren wrote: >> >> So let's change the project-wide coding rules instead to allow 99 >> >>> columns as the hard limit, but keep 80 columns as the recommended >>> (soft) limit. >>> >>> >> I think we should avoid opening up a can of worms on the merits of >> different styles, and instead focus on the most pragmatic ways to unify >> Gecko and JS style. Under that framework, Mats' proposal makes a lot of >> sense. >> >> > I do not see the advantages of having huge patches to rewrite an entire > project just for the benefit of having only one style guide. > This was discussed extensively in [1]. I'd be happy to re-discuss the merits with you sometime off-list, but the outcome of the thread was that all the relevant module owners (including jorendorff and myself) agreed that unifying style guides was an important goal. So let's just take that goal as given and avoid getting sucked into rehashing that other thread. [1] https://lists.mozilla.org/pipermail/dev-platform/2014-January/002676.html ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: SpiderMonkey and XPConnect style changing from |T *p| to |T* p|
> and security issues as contributors (including employees) > do not look for the original authors. It would be really nice if there was a way to annotate whitespace-only changesets as being such, and then configure tools such as "hg blame" to "look past" them. Cheers, Botond ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: SpiderMonkey and XPConnect style changing from |T *p| to |T* p|
On Fri, Mar 27, 2015 at 7:59 PM, Botond Ballo wrote: > It would be really nice if there was a way to annotate whitespace-only > changesets as being such, and then configure tools such as "hg blame" > to "look past" them. You mean `hg blame -w` ? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
