Followup: modularity, WebExtensions, and going faster

Fri Oct 7 16:47:18 UTC 2016

On Fri, Oct 7, 2016 at 9:11 AM, Mike de Boer <mdeboer at mozilla.com> wrote:

> I think Benjamin’s first point that swiftly attempts to validate the
> assumption that the core limiting factor is in fact the modularity and
> module boundaries in our codebase is comparing apples to oranges by
> offsetting process against code architecture.
> I think we’re looking for a problem that presumably causes our apparent
> inability to go faster in the wrong area.
> In that vein I was very pleased with Richard Newman’s post to this thread;
> you should read it!
>

You make a lot of good points about the need for a culture that encourages
refactoring, Mike. I think it would make sense to bring this up in a
separate email thread. I'd like to agree with you more, except that this
thread is about WebExtensions, and I disagree with you on that point :-).

> Before I start, I’d like to point out that our current WebExtensions model
> is mostly comprised of interfacing with our lower level JS modules, adding
> a layer of indirection that we most often don’t need in frontend code that
> builds the browser.
>

Yes, the WebExtensions API won't magically make our code great. But it's a
tool, and I do think we need better tools. We wouldn't want to write the
browser in assembly, after all. Fundamentally, WebExtensions gives us a
typesafe API mechanism. If you haven't seen them yet, you might be
interested in the schema files that define WebExtension interfaces. Here's
an example:
http://searchfox.org/mozilla-central/source/browser/components/extensions/schemas/tabs.json
>From files like this, we automatically check the types of every method
parameter. We can also generate documentation automatically (and you can
see those docs in MDN). There's also a simple, uniform way of reporting and
handling errors.

WebExtensions also enforces separation more strongly than any other
mechanism we have now. In theory each extension can run in a separate
process, so all communication between them has to go through well-defined
APIs.

None of this is necessary to write good code, but it certainly helps.

That said, I agree with you that we need to be better about refactoring our
code. We definitely shouldn't wait for a Grand Browser Rewrite to fix all
the bad stuff we have. In modules where someone has been ambitious about
improving the code (and I think Tim Taubert's work on session store is a
great example) we've managed to turn gross code into great code through
gradual refactoring. We should encourage more of that!

-Bill

> Our current development model skews big code refactors and favours small,
> incremental iterations that extend the existing code a bit where it fits
> best, adds a unit test and call it a day.
> I hear more and more noises, coming from various directions (product, UX,
> devtools, addon-sdk, WebExtensions, Servo, B2G, etc.) that are all
> variations of “Hey, what if we could rewrite Firefox to get rid of all this
> old, complicated crap?”
> This should not be taken seriously. At least, not in the sense that we
> should actually _try_ to rewrite Firefox in its entirety. It’s in fact a
> big fat billboard poster on the wall in front of you that says: “This
> culture sucks, I don’t feel empowered enough to accomplish anything of
> sufficient value here!”, “Something needs to change! I don’t know what
> exactly, so rewriting the entire thing is what I can come up with right
> now!” and “Halp!”
> If you read between the lines and talk to all developers - not just the
> ones who are loud - you’ll have to notice that the current cultural setting
> is not enabling us to go faster at all.
> Rewriting the browser: not faster. Making small changes with le unit test
> attached: not faster. Adding a layer on top of existing modules: not faster.
> What’s missing is that we are not able to _sell_ code refactoring, thus
> we’re almost never doing it, unless it’s absolutely necessary. Like when
> Electrolysis happened. (And moving to multiple content processes is going
> to yield even more refactoring and modularisation of code into a more
> modern style.) And if we’re not doing it enough, we don’t get a chance to
> become really good at it, either.
> We’re scared to make changes other than small ones, because of their
> high(er) potential to introduce regressions. Regressions have a bad name,
> because they cause more work, keep developers busy whilst the calendar says
> they’re supposed to be working on something else. Managers hate them,
> because one cannot plan ahead whilst all these issues take priority over
> all other (future) work. Release Engineering will bounce your uplift
> requests and push for more strictness. I’d like to call them side-effects,
> instead, because they’re very valuable:
>  * It shows our community from their best and shiniest angle: reporting
> bugs! Why focus on trying to move this colossus of a body to something that
> they don’t want to do (write code), but instead capitalise on their
> oft-emerging strength (file bugs) to feel a part of the community and help
> the Fox forward?
>  * They uncover parts of the codebase that are, yes, not covered by unit
> tests.
>  * When fixed, they are _always_ verified by our community members who
> filed the bug, showing yet another shiny angle from them.
>
> But even though side-effects have these fine advantages, we wary them,
> because they’re not part of the process. If you don’t have a good method of
> dealing with side-effects, good luck pulling a code refactor through
> without a bruise or two.
> But fortunately for us, process has a tendency to evolve when the need
> arises (again, look at e10s), so the only thing stopping us from moving
> forward and faster is our inability to sell code refactors.
> Why would we need to remove #ifdef’s in our CSS and JS code _right now_?
> Why do we need to split browser.js or nsSearchService.js up into modules
> _right now_? It’s never a priority, until the collective frustration - not
> of the developers _working on_ it, but of all the developers having to
> _read_ it - reaches the boiling point. We seem to getting closer nowadays,
> is the feeling I get when I occasionally put out my temperature gauge.
> Of course, there are strict rules attached to doing a code refactor of
> reasonable size and that’s where senior mentorship (note that I’m not using
> the prefix _leader_ at all) comes in to guide these projects and teams in
> the right direction with tips and best practices or even contributions.
> It’s also their responsibility to coordinate efforts and make sure there’s
> a certain gradual approach to refactoring the frontend, like not running
> two tracks at the same time.
>
> Another, closely related defect that’s highly likely to have an effect on
> our inability to move faster is our culture of module ownership. I’m
> advocating for a complete removal of that concept. The word ‘owner’ has too
> many connotations that confuse the heck out of people: “Do you own that
> code like you own a house? What does that mean? Do I need to ask your
> permission if I want to step on your lawn?“
> Instead, the current module ‘owners’ list should be reset to being
> ‘candidate reviewers’ only; people who have enough understanding of that
> area of functionality that they’re able to make qualitative judgments on
> contributions, are available for any question a contributor might have and
> willingly partake in triage sessions. (The fact that developers do triage
> in the first place is an interesting subject to me, but out of scope here.)
> There is always someone who must _own_ the project. But in that context,
> the word simply fits. This person must assure that there are always enough
> candidate reviewers available for the different functional areas.
>
> When you think and talk about modules and architecture, I think that
> simple faith in the fact that a smart, top-notch developer, like we all are
> of course, has a natural tendency to take the modern approach to writing
> new code. I mean, when you look at the most recently written slabs of code,
> I see the latest ECMAScript features, ReactJS, async-await style control
> flows, less LOC per module and smaller function bodies. Technically, we
> ought to be able to refactor very well; it shows.
>
> I think that I’m at a word count currently that’s more than enough for
> now. I did not permit myself the luxury of a proof-read, so please forgive
> the broken English if it’s there. Please feel free to reach out if you feel
> I should clarify/ say more!
>
> Cheers,
>
> Mike.
>
> On 06 Oct 2016, at 17:08, Benjamin Smedberg <benjamin at smedbergs.us> wrote:
>
> I spent a week writing a thing about modularity, webextensions, and going
> faster. I think it's important for us to decide the module structure of our
> code especially as we start shipping independent modules/going faster. And
> I believe that having better module structure, boundaries, and
> documentation is critical to our teams being more agile and also attracting
> contributors to the project.
>
> http://benjamin.smedbergs.us/blog/2016-09-03/modularity-and-webextensions/
>
> I personally think that we should double down on WebExtensions as a model
> and start using that for large parts of Firefox. But Andy McKay and Rob
> Helper had some good counter-thoughts and I've asked them to post here to
> elaborate.
>
> In the post I asked everyone to send followups to firefox-dev, so I wanted
> to start a thread here to collect responses. Over the next months I'd like
> this to turn into a firm decision about how we're going to build system
> addons; but I'd like to start by seeing what feedback people have and even
> whether I've framed the problem correctly.
>
> --BDS
> _______________________________________________
> firefox-dev mailing list
> firefox-dev at mozilla.org
> https://mail.mozilla.org/listinfo/firefox-dev
>
>
>
> _______________________________________________
> firefox-dev mailing list
> firefox-dev at mozilla.org
> https://mail.mozilla.org/listinfo/firefox-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/firefox-dev/attachments/20161007/8df83c1d/attachment.html>