Blink Code Style

91 views
Skip to first unread message

Sasha Bermeister

unread,
Jul 25, 2016, 9:57:09 PM7/25/16
to blink-dev, Dana Jansens, Elliott Sprehn, Nico Weber, jyas...@chromium.org
Hi blink-dev,

It's been a little ambiguous what's happening to our code style after the merge. There were discussions about Converging with Chromium Style (and revision 2) that ended over 6 months ago and various rename patches to prevent conflicts post-rename over 4 months ago.

As a developer on the blink code who is about to clean up and add a bunch of classes, what style do I use? Are any of the style guide documents up-to-date?

Just wanted to clear up the confusion. :)

Thanks,

Sasha

dan...@chromium.org

unread,
Jul 25, 2016, 10:17:11 PM7/25/16
to Sasha Bermeister, blink-dev, Elliott Sprehn, Nico Weber, Jeffrey Yasskin
Hi Sasha,

Thanks for asking, I've been meaning to send an email and this prompted me :)

dcheng and I were doing the work of making a tool to do the conversion to blink style, but we've both been caught up with other things so the project has stalled for a bit.

This would make a great 20% project if anyone wants to contribute. This is the tracking bug:


As you can see there are currently 12 known issues that need to be resolved. We found most of them by running the tool and then looking at the resulting compiler errors. It would be great to get some more volunteers to help tackle these bugs and get the tool working so we can get into a more uniform state.

As it is, blink code should stay blink style for now because inconsistency within a file is pretty. Though new files moved from chromium have been left in chromium style I think as it's kinda silly to do the work of reformatting them just to have the tool put them back.

- Dana

Nico Weber

unread,
Jul 29, 2016, 7:04:04 PM7/29/16
to Sasha Bermeister, blink-dev, Dana Jansens, Elliott Sprehn, Jeffrey Yasskin
We discussed this a bit at the last blink-on.

There are (at least) two parts to this project.

1. Changing method cases etc like Dana says
2. Whitespace-only changes

For 2, the major blocker was to come up with a way to do this without causing everyone merge conflicts for their local branches. During blinkon, we came up with a solution that we think should make rebasing your local branches just work without any manual work (see https://crbug.com/574611). I want to add this to depot_tools soon (I've been on vacation the last two weeks), and that part should hopefully make progress in the near future.

Emil A Eklund

unread,
Aug 1, 2016, 1:16:58 PM8/1/16
to Nico Weber, Sasha Bermeister, blink-dev, Dana Jansens, Elliott Sprehn, Jeffrey Yasskin
>> It's been a little ambiguous what's happening to our code style after the
>> merge. There were discussions about Converging with Chromium Style (and
>> revision 2) that ended over 6 months ago and various rename patches to
>> prevent conflicts post-rename over 4 months ago.

On that note. For entirely new blink code (new components) should we
try to conform to Chromium style or stick with blink style for now?

TAMURA, Kent

unread,
Aug 1, 2016, 9:32:25 PM8/1/16
to Emil A Eklund, Nico Weber, Sasha Bermeister, blink-dev, Dana Jansens, Elliott Sprehn, Jeffrey Yasskin
I think we need to keep the current WebKit-based Blink style for implementations of web-exposed interfaces until the great reformatting, and it's ok to start using Chromium-based Blink style for internal classes such as WTF, platform, layout and paint.
--
TAMURA Kent
Software Engineer, Google


Kentaro Hara

unread,
Aug 2, 2016, 1:14:08 AM8/2/16
to TAMURA, Kent, Emil A Eklund, Nico Weber, Sasha Bermeister, blink-dev, Dana Jansens, Elliott Sprehn, Jeffrey Yasskin
I'd prefer not introducing any rule until we finish the conversion; i.e., new code can use either the WebKit-based Blink style or the Chromium-based Blink style.

If you're migrating code from platform/ to Blink, the Chromium-based Blink style would be easier. If you're adding new code to existing files, the WebKit-based Blink style would be easier.





--
Kentaro Hara, Tokyo, Japan
Reply all
Reply to author
Forward
0 new messages