Reformatting the comm-central C++ code base to Google coding style
Hello,
as you may know, M-C have reformatted their entire C++ code base[1] and
are continuing to do so[2]. They even reformatted the mozilla-esr60 branch.
I intend to do the same for the comm-central code base and now is a good
time to start to get it all done before the end of the 68 cycle so that
future updates to comm-esr68 will be easy.
Unless I hear a resounding "don't do it", I will start now with
low-traffic directories, like
mail/ - Very little C++ code there
ldap/
db/
common/
then slowly moving through mailnews/, again starting with low-traffic
directories like db, extensions, import, intl, jsaccount, mapi so that
no C++ patches which are in development will be invalidated. I see that
the remaining de-RDF patches have little C++ content in them, so even if
they don't land within the 68 cycle, it wouldn't be a big problem.
Comments?
If you want to try it out: mach clang-format -p comm/dir/of/your/choice,
issued from the M-C top source directory.
Jörg.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1188202
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1519636
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1513900
Do it! Do you have a bug for the c-c work? I'd like to include calendar minus libical.
Philipp
On 23. Apr 2019, at 10:25 AM, Jörg Knobloch jorgk@jorgk.com wrote:
Hello,
as you may know, M-C have reformatted their entire C++ code base[1] and are continuing to do so[2]. They even reformatted the mozilla-esr60 branch.
I intend to do the same for the comm-central code base and now is a good time to start to get it all done before the end of the 68 cycle so that future updates to comm-esr68 will be easy.
Unless I hear a resounding "don't do it", I will start now with low-traffic directories, like
mail/ - Very little C++ code there
ldap/
db/
common/
then slowly moving through mailnews/, again starting with low-traffic directories like db, extensions, import, intl, jsaccount, mapi so that no C++ patches which are in development will be invalidated. I see that the remaining de-RDF patches have little C++ content in them, so even if they don't land within the 68 cycle, it wouldn't be a big problem.
Comments?
If you want to try it out: mach clang-format -p comm/dir/of/your/choice, issued from the M-C top source directory.
Jörg.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1188202
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1519636
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1513900
Maildev mailing list
Maildev@lists.thunderbird.net
http://lists.thunderbird.net/mailman/listinfo/maildev_lists.thunderbird.net
On 23/04/2019 11:44, Philipp Kewisch wrote:
Do it! Do you have a bug for the c-c work? I'd like to include calendar minus libical.
I filed a bug now: https://bugzilla.mozilla.org/show_bug.cgi?id=1546364
Jörg.
On 23/04/19 20:25, Jörg Knobloch wrote:
[snip]
I see that
the remaining de-RDF patches have little C++ content in them, so even if
they don't land within the 68 cycle, it wouldn't be a big problem.
There are a bunch of C++ de-RDF patches waiting to land. They don't work
with feeds. There are de-RDF patches for the feeds code, one has been
OKed, the other still waiting on review (sounds like it'll probably be a
week or two due to the size of the patch and to travel plans).
But I don't think this should hold up progress - I say go ahead and
start on the reformatting.
I'll make sure any outstanding de-RDF patches are updated accordingly.
If you want to try it out: mach clang-format -p comm/dir/of/your/choice,
issued from the M-C top source directory.
The thought of a automatically reformatting a big C++ codebase like this
is simultaneously awesome and terrifying!
Ben.
On 4/23/19 11:34 PM, Ben Campbell wrote:
On 23/04/19 20:25, Jörg Knobloch wrote:
[snip]
I see that the remaining de-RDF patches have little C++ content in
them, so even if they don't land within the 68 cycle, it wouldn't be
a big problem.
There are a bunch of C++ de-RDF patches waiting to land. They don't
work with feeds. There are de-RDF patches for the feeds code, one has
been OKed, the other still waiting on review (sounds like it'll
probably be a week or two due to the size of the patch and to travel
plans).
But I don't think this should hold up progress - I say go ahead and
start on the reformatting.
I'll make sure any outstanding de-RDF patches are updated accordingly.
Note I recall when m-c announced this they had some magic way to that
patches could be rebased with zero effort despite the formatting
changes. You may want to dig up their announcement to find out how. I
think it was something about adding clang-format to the hg config in
some way, then it would be used during rebases to make it look correct.
Philipp
On 24/04/19 11:29, Philipp Kewisch wrote:
On 4/23/19 11:34 PM, Ben Campbell wrote:
I'll make sure any outstanding de-RDF patches are updated accordingly.
Note I recall when m-c announced this they had some magic way to that
patches could be rebased with zero effort despite the formatting
changes. You may want to dig up their announcement to find out how. I
think it was something about adding clang-format to the hg config in
some way, then it would be used during rebases to make it look correct.
It should be OK in the rdf-removal case. I got mixed up and forgot that
most of the C++ patches had already landed. The bulk of the remaining
stuff is JS, so Jörg's initial assessment was right.
Ben.
Hi,
Finally sane formatting done by mechanical tool, and not by human
intervention.
I would suggest you announce a "flag day" where all the code will be
reformatted.
This way, would-be patch creators will work on the
new format if they know that their patch will be likely to be
incorporated in the tree
AFTER the reformatting.
This way, they would be ready and WON'T BE SURPRISED.
I think linux and much, much earlier TENEX/TWENEX used similar
approaches for
system-wide changes such as changes of character codes (ugh).
Just my two cents worth.
Chiaki
n 2019/04/23 17:25, Jörg Knobloch wrote:
Hello,
as you may know, M-C have reformatted their entire C++ code base[1]
and are continuing to do so[2]. They even reformatted the
mozilla-esr60 branch.
I intend to do the same for the comm-central code base and now is a
good time to start to get it all done before the end of the 68 cycle
so that future updates to comm-esr68 will be easy.
Unless I hear a resounding "don't do it", I will start now with
low-traffic directories, like
mail/ - Very little C++ code there
ldap/
db/
common/
then slowly moving through mailnews/, again starting with low-traffic
directories like db, extensions, import, intl, jsaccount, mapi so that
no C++ patches which are in development will be invalidated. I see
that the remaining de-RDF patches have little C++ content in them, so
even if they don't land within the 68 cycle, it wouldn't be a big
problem.
Comments?
If you want to try it out: mach clang-format -p
comm/dir/of/your/choice, issued from the M-C top source directory.
Jörg.
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1188202
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1519636
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1513900
Maildev mailing list
Maildev@lists.thunderbird.net
http://lists.thunderbird.net/mailman/listinfo/maildev_lists.thunderbird.net
On 2019/04/24 8:29, Philipp Kewisch wrote:
On 4/23/19 11:34 PM, Ben Campbell wrote:
On 23/04/19 20:25, Jörg Knobloch wrote:
[snip]
I see that the remaining de-RDF patches have little C++ content in
them, so even if they don't land within the 68 cycle, it wouldn't be
a big problem.
There are a bunch of C++ de-RDF patches waiting to land. They don't
work with feeds. There are de-RDF patches for the feeds code, one has
been OKed, the other still waiting on review (sounds like it'll
probably be a week or two due to the size of the patch and to travel
plans).
But I don't think this should hold up progress - I say go ahead and
start on the reformatting.
I'll make sure any outstanding de-RDF patches are updated accordingly.
Note I recall when m-c announced this they had some magic way to that
patches could be rebased with zero effort despite the formatting
changes. You may want to dig up their announcement to find out how. I
think it was something about adding clang-format to the hg config in
some way, then it would be used during rebases to make it look correct.
Philipp
Such an automagical conversion to fit a patch to new reformatting style
will be awesome.
Chiaki
Right on, and yay for automated formatting!
-Paul
(P.S. Maybe, someday, perhaps, JavaScript too...? I've found using
Prettier (https://prettier.io/) to be quite nice.)
On 24/04/2019 01:29, Philipp Kewisch wrote:
Note I recall when m-c announced this they had some magic way to that
patches could be rebased with zero effort despite the formatting
changes. You may want to dig up their announcement to find out how. I
think it was something about adding clang-format to the hg config in
some way, then it would be used during rebases to make it look correct.
Well, we don't have so many patches on the go, and sadly "zero effort"
means setting up the tools first :-(
Jörg.