Base is "done": please stop adding crap
Brett Wilson
- When adding new code, put it in the most specific location that
makes sense (usually not base).
- Avoid *_util files.
True story:
About 2 years ago somebody (I've forgotten who, which is good for
them) sent out a change to add a function to base/string_util. The
function was "interpret the input string in argument 1 as a series of
tokens separated by the character given in argument 2, and parse it as
key/value pairs, interpreting every-other-one as an integer, returning
the result in output argument 3 as a vector<pair<string, int>>". I
asked why they were adding such a crazy function to base, and they
said "so somebody can re-use this code."
The size of our code base is itself one of the largest impediments to
code re-use. Even if the function you want exists in string_util or in
some other file in base, you'll have a hard time finding it because of
all the other "useful" functions/files in the way. Taken to an
extreme, the entire browser should be in base because you should be
able to use it in different products.
The current code in base has supported a a very large stable browser
project for many years. It's exceedingly likely that if some concept
isn't already in base, it's not needed, and the team overall can
benefit from having a more focused list of core concepts. While there
will always be some exceptions, everybody's default position should be
to be skeptical of new functionality in base.
So where should you put your code? Put it in the most specific place
that makes sense. We can always move it later if it ends up being
really awesome.
- In your component's directory. Be realistic about the likelihood
that somebody in another component will want your functionality,
actually find your code (even if it's in base), and use your solution.
If the answer is "low", just put it in your directory.
- chrome/common (shared by the entire product) and chrome/browser
(browser process only).
- app. This is "base for windowed applications." Common code dealing
with the window manager, and clipboard, and user input/output goes
here. Try to avoid too-specific-stuff for the same reasons as for
base.
- base in an appropriate subdirectory. We've been adding more
categorization to make it easier to find stuff that really needs to be
shared across many projects.
- base in the root. Your code is so great everybody should be hit over
the head with it.
BUT base still needs a lot of cleanup and improvement, and this
doesn't mean we don't want changes. If you code really does clean up
different places across our codebase and you actually do this cleanup,
it's something we want shared as widely as possible.
Brett
David Levin
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
Brett Wilson
> What recent addition triggered this? Are you planning to move things out of
> base?
I have moved about 100 files out of the root base directory in the
past month. Some of these went into subdirectories in base. Some have
gone into the other places I listed in the email. I've moved about
30-50 files out of base before that in the past year.
> The "true story" which supports this is a strawman scenario.
The story was intended to be an illuminating example rather than
justification for a change ("somebody did X two years ago so now we
have a new rule Y."). I think this was pretty clear.
> Base looks like a framework of utility functions and classes. These grow
> over time as a code base develops new patterns and new ways of doing things.
> (Look at any framework as an example.) Stopping this seems to imply that
> things should not change to do things in new ways.
Any good framework provides some limits on the types of things that
you can do or they would explode. STL provides generic ways of
expressing new types of algorithms and containers and doesn't add
every kind of random crap that people propose. New stuff gets added
only when its generally useful to a large number of applications. This
is the model I (fairly clearly, I think) explained.
Brett
Ben Goodger (Google)
- app. This is "base for windowed applications." Common code dealing
with the window manager, and clipboard, and user input/output goes
here. Try to avoid too-specific-stuff for the same reasons as for
base.
David Levin
Brett Wilson
> I understand now and that makes sense. I read "crap" as stuff not as crap.
> :)
> The subject is slightly misleading 'Base is "done"' which is what led to my
> misunderstanding. I believe understand that too (largely done and the bar is
> high for adding things there because low hanging fruit has been done).
I purposely used an inflammatory subject line so people would read my email :)
Brett
James Robinson
Brett Wilson
I forgot to mention the _util files. When possible, please file the
most specific place to put your code, and use the most specific name
possible. Historically we've had a number of foo_util files which are
a dumping ground for lots of random stuff related to foo (string_util,
win_util, etc.). If you really have some random crap, try to make a
specific util file rather than a general one. Example: the stuff
converting strings to numbers was split off iof string_util nto
string_number_conversions.h, and the stuff dealing with HWND was moved
from win_util to hwnd_util.
Brett