PSA: Blink Rename is happening on April 8

[Daniel Cheng]

Hello blink community:

On April 8th, we will use tooling to rename Blink identifiers to match the guidance in the New Blink Style Guide [1]. We will be closing the tree at 9 AM PST and expect to have the tree reopened by 7 PM PST.

We've prepared tooling to help developers migrate branches across the rename; unfortunately, given the complexities of this change, the rebasing process will be guided and interactive, rather than completely automated like the clang-format changes. If you encounter difficulties trying to resolve merge conflicts, please reach out to us.

Unless you care about the details, you can stop reading now.

What’s being renamed:

- Fields: m_myFieldName => my_field_name_

- Variables: myVariableName => my_variable_name

- Functions/methods: myFunctionName => MyFunctionName

- Enumerators: EnumValue => kEnumValue

Note that Blink will not be using hacker_case for simple getters and setters. Methods will always be CasedLikeThis(), even if it’s a simple inlined getter/setter.

Naming exceptions:

- Web-exposed methods will remain in lowerCamelCase(). Document::open(), DOMWindow::closed(), Node::parentNode(), etc will retain their existing naming. Anything exposed via WebIDL will retain the lower camel case naming style.

- Renamed function that collide with type names will be prefixed with “Get”, e.g. DOMWindow::frame() will become DOMWindow::GetFrame().

- Enumerators that are in SHOUTY_CASE will not be renamed.

- dcheng@, on behalf of the Blink renaming cabal

[1] https://docs.google.com/document/d/1ZQLiQKw9hCOyDDhDS4JhckhuiJmLqSbw9FLfySMyxgU/edit

[Dimitri Glazkov]

It's happening!!!

[Daniel Cheng]

+chromium-dev, since the tree closure will affect Chromium developers as well

Daniel

[Daniel Cheng]

As an update, it looks unlikely that the tree will reopen by 7 PM. New ETA is midnight. Sorry for the trouble!

Daniel

[Daniel Cheng]

This is all landed now, and the builders should be green (or going green shortly). I'll be deploying the rebase helpers and sending out instructions on using it in a few hours after final config pushes / testing. Thanks!

Daniel

[Emil A Eklund]

Awesome, thank you for working on this!

[Sami Kyostila]

It's such a relief not having to worry about two different coding styles anymore -- thanks for doing this!

I noticed some constant that were already kChromiumStyle got rewritten as k_hacker_style (example). I guess that wasn't expected? Should we just fix these up manually?

- Sami

ma 10. huhtik. 2017 klo 6.20 Emil A Eklund <e...@chromium.org> kirjoitti:

Awesome, thank you for working on this!

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Nico Weber]

Heh, that wasn't intentional. Looks like those were local constants. Luckily, there aren't too many of them: https://cs.chromium.org/search/?q=%5Cbk_+file:third_party/webkit&type=cs I'll fix them up.

- Sami

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

[Nico Weber]

On Mon, Apr 10, 2017 at 10:18 AM, Nico Weber <tha...@chromium.org> wrote:

Heh, that wasn't intentional. Looks like those were local constants. Luckily, there aren't too many of them: https://cs.chromium.org/search/?q=%5Cbk_+file:third_party/webkit&type=cs I'll fix them up.

(https://codereview.chromium.org/2812643002/)

[Daniel Cheng]

Thanks for fixing these. There's a few more known issues (see the blockers on https://bugs.chromium.org/p/chromium/issues/detail?id=675877). Please file bugs for anything else you come across, and we'll make sure to fix them.

Daniel

- Sami

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Hiroshige Hayashizaki]

Hi,

> We've prepared tooling to help developers migrate branches across the rename

Could you provide the details of the tooling? e.g. how to run, what can be done automatically and what should be done manually when rebasing branches?

Thanks!

Daniel

- Sami

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

[Chris Blume]

As a head's up, it updated active variable names but not variable names in comments.
For example:

// |m_alphaSeen| will blah blah

bool alpha_seen_;

I have been manually updating the comments in my area.

Chris Blume | Software Engineer | cbl...@google.com | +1-614-929-9221

[Chris Blume]

Oh sorry. I see that is being tracked in a bug. Ignore my previous email :D

Chris Blume | Software Engineer | cbl...@google.com | +1-614-929-9221

[Daniel Cheng]

I've pushed the beta version of the rebase helper out. Due to the way we executed the rename, the rebase helper requires *building* some files when resolving conflicts. I probably wouldn't use it for a 5 line CL, but it should be helpful for larger CLs.

Caveats:

• If you have dependent branches, you must run --prepare on *all* the dependent branches before running --update. You may also encounter some bugs with dependent branches (my recollection is git rebase mostly works, but sometimes you need --onto as well? The tool currently doesn't pass --onto.)
  
• The tool requires that all commits in the branch be squashed into one.
  
• The tool assumes if you have a tracking branch set. git cl assumes that you're tracking origin/master by default if you have nothing set; if that's the case for you, you can set the upstream branch by running git branch --set-upstream-to origin/master
  
• There's a known bug with the tool where some names don't get updated. This has to do with the fact that 'bool' is a macro (!!!). We'll try to fix this, but until it's fixed, please do inspect the merged result with some caution.
  

Setup instructions:

If you don't have tools/blink_rename_merge_helper/run.py in your repository, you can stage a copy by running:

# Note that this intentionally saves it into run2.py to avoid conflicting with the checked in copy of run.py.

git fetch origin

git show cff45fb322cbdf2badb6ededde0106161bbfb13e > tools/blink_rename_merge_helper/run2.py

Then use run2.py in the instructions below, rather than run.py.

Updating branches:

# This rebases the current branch up to right before the rename commit,

# then runs the clang tool and records the result for future conflict

# resolution.

# Optional arguments: -j <N> to use more than ninja's default number

# of parallel processes.

tools/blink_rename_merge_helper/run.py -C <build_dir> --prepare

# Only needed if manual conflict resolution was needed in the previous step

# Note that any changes in the working tree should not be committed;

# they are recorded and reverted by this step.

tools/blink_rename_merge_helper/run.py --finish-prepare

# Rebases the current branch to the rename commit.

# Uses the files generated from --prepare / --finish-prepare

# to do conflict resolution.

tools/blink_rename_merge_helper/run.py --update

Optional arguments for --prepare and --update: 

--merge: If you update your branches by merge. Completely untested, since I use a rebase workflow. I recommend not using this.

If you have any issues using this tool or need help with any large patches, please let me know.

Daniel

Daniel

- Sami

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Dominic Mazzoni]

Thanks for doing a great job with the tooling! blink_rename_merge_helper worked well for me on a 500-line change. I had no merge/rebase conflicts, I just had to manually rename a few functions and variables I introduced in my patch.

[Kouhei Ueno]

Hi,

Thanks for driving this work. While we believe this was carried out great overall, we did experience some difficulties from the change, so I wanted to make it aware.

Web platform devs in APAC timezone / who work over weekends (probably both full-time and hobbyists) had to spend 0.5 - 3 days just to rebase CLs.

I think we can improve this by:

- Making sure that the rebase helper is available before the change. The helper was not available at the time I needed rebase, so I manually rebased >1.5k LoC local changes.

- Make instructions for locally running the renamer obvious. It took us a while to find https://cs.chromium.org/chromium/src/docs/clang_tool_refactoring.md?sq=package:chromium&dr

Daniel

- Sami

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

--

kouhei

[Daniel Cheng]

On Sun, Apr 16, 2017 at 8:41 PM Kouhei Ueno <kou...@google.com> wrote:

Hi,

Thanks for driving this work. While we believe this was carried out great overall, we did experience some difficulties from the change, so I wanted to make it aware.

Web platform devs in APAC timezone / who work over weekends (probably both full-time and hobbyists) had to spend 0.5 - 3 days just to rebase CLs.

I think we can improve this by:

- Making sure that the rebase helper is available before the change. The helper was not available at the time I needed rebase, so I manually rebased >1.5k LoC local changes.

Hi kouhei@,

Sorry for the trouble with the rebase helper: though it was prepared beforehand, I noticed an issue (https://crbug.com/711128) while verifying correctness after the big CL landed on Sunday morning. Since I did not understand why it was not working as expected,  I did not immediately push the tool in case it would destroy developer's branches.

That being said, the rebase helper was available by Tuesday morning APAC time, so that should have been only 1 day when the rebase helper was not available in APAC. Were there other issues using the rebase helper after it was released?

 

- Make instructions for locally running the renamer obvious. It took us a while to find https://cs.chromium.org/chromium/src/docs/clang_tool_refactoring.md?sq=package:chromium&dr

This is a good suggestion, which I'll make sure to include for any future work that involves clang tooling like this.

Daniel

 

Daniel

- Sami

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

--

kouhei

[Kouhei Ueno]

Hi dcheng@,

Thanks for driving this!

On Mon, Apr 17, 2017 at 1:14 PM, Daniel Cheng <dch...@chromium.org> wrote:

On Sun, Apr 16, 2017 at 8:41 PM Kouhei Ueno <kou...@google.com> wrote:

Hi,

Thanks for driving this work. While we believe this was carried out great overall, we did experience some difficulties from the change, so I wanted to make it aware.

Web platform devs in APAC timezone / who work over weekends (probably both full-time and hobbyists) had to spend 0.5 - 3 days just to rebase CLs.

I think we can improve this by:

- Making sure that the rebase helper is available before the change. The helper was not available at the time I needed rebase, so I manually rebased >1.5k LoC local changes.

Hi kouhei@,

Sorry for the trouble with the rebase helper: though it was prepared beforehand, I noticed an issue (https://crbug.com/711128) while verifying correctness after the big CL landed on Sunday morning. Since I did not understand why it was not working as expected,  I did not immediately push the tool in case it would destroy developer's branches.

That being said, the rebase helper was available by Tuesday morning APAC time, so that should have been only 1 day when the rebase helper was not available in APAC. Were there other issues using the rebase helper after it was released?

I didn't get the chance to use the helper, so I can't provide the insight there.

Daniel

- Sami

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

--

kouhei

--

kouhei

[Philip Jägenstedt]

Thanks for the rebase helper! I also first had the need last Monday (APAC) thought it wouldn't be too hard to just reformat using the clang tool with my patch applied, but I couldn't get that to work.

The rebase helper did work for one rather big revert I needed to get across the rename. But then I had another dependent CL that deleted some fails, and that caused the script to fail inside some python code. That's probably a bug any time a rewritten file is deleted, can you take a look?

[foo...@google.com]

Actually, I noticed later it didn't work flawlessly even for my first CL. Any file that I had touched that had some parts of code behind ifdefs not build on my machine would effectively revert those sections to the pre-rename style. The result still built for me of course, but it required a fair bit of git checkout -p to revert the spurious changes.

[Daniel Cheng]

On Mon, Apr 17, 2017 at 10:01 PM <foo...@google.com> wrote:

Actually, I noticed later it didn't work flawlessly even for my first CL. Any file that I had touched that had some parts of code behind ifdefs not build on my machine would effectively revert those sections to the pre-rename style. The result still built for me of course, but it required a fair bit of git checkout -p to revert the spurious changes.

This is a limitation of clang tooling - it can't reformat code it doesn't know about. =/

 

On Monday, April 17, 2017 at 7:12:20 PM UTC+7, Philip Jägenstedt wrote:

Thanks for the rebase helper! I also first had the need last Monday (APAC) thought it wouldn't be too hard to just reformat using the clang tool with my patch applied, but I couldn't get that to work.

The rebase helper did work for one rather big revert I needed to get across the rename. But then I had another dependent CL that deleted some fails, and that caused the script to fail inside some python code. That's probably a bug any time a rewritten file is deleted, can you take a look?

Just to clarify, were your branches structured like this:

origin/master <--- revert branch <--- "deletes fails" branch

If so, was the revert branch rebased past the rename commit before starting the rebasing process for the "delete fails" branch?

Daniel

[Philip Jägenstedt]

Actually, I noticed later it didn't work flawlessly even for my first CL. Any file that I had touched that had some parts of code behind ifdefs not build on my machine would effectively revert those sections to the pre-rename style. The result still built for me of course, but it required a fair bit of git checkout -p to revert the spurious changes.

[Mounir Lamouri]

Piling up on this thread to share some feedback. The rename happened
over the week-end and my understanding is that the rewrite tool landed
on Monday morning US Pacific time. When I went to the office the Monday
morning after the Big Rename, I had to manually rebase a few refactoring
CLs. That was very costly. Arguably, I could have waited for a tool to
be available but that meant leaving these CLs untouched for most of the
working day.

Maybe in the future it would good to make sure that the required tools
have landed when the changes are made and not the next morning given
that "next morning" means that EMEA and APAC working days are (mostly)
done.

I do understand that you have worked over the week-end to make this
change and I'm very glad it happened :) I'm only sharing this feedback
because I saw in a different thread that Nico was surprised that someone
(from Tokyo) had to manually rebase.

-- Mounir

On Mon, 17 Apr 2017, at 22:17, Philip Jägenstedt wrote:
> Actually, I noticed later it didn't work flawlessly even for my first CL.
> Any file that I had touched that had some parts of code behind ifdefs not
> build on my machine would effectively revert those sections to the
> pre-rename style. The result still built for me of course, but it
> required
> a fair bit of git checkout -p to revert the spurious changes.
>
> On Mon, Apr 17, 2017 at 7:12 PM Philip Jägenstedt <foo...@chromium.org>
> wrote:
>
> > Thanks for the rebase helper! I also first had the need last Monday (APAC)
> > thought it wouldn't be too hard to just reformat using the clang tool with
> > my patch applied, but I couldn't get that to work.
> >
> > The rebase helper did work for one rather big revert I needed to get
> > across the rename. But then I had another dependent CL that deleted some
> > fails, and that caused the script to fail inside some python code. That's
> > probably a bug any time a rewritten file is deleted, can you take a look?
> >
> >
> > On Tue, Apr 11, 2017 at 7:01 AM Daniel Cheng <dch...@chromium.org> wrote:
> >
> >> I've pushed the beta version of the rebase helper out. Due to the way we
> >> executed the rename, the rebase helper requires *building* some files when
> >> resolving conflicts. I probably wouldn't use it for a 5 line CL, but it
> >> should be helpful for larger CLs.
> >>

> >> *Caveats:*
> >>
> >> - If you have dependent branches, you must run --prepare on *all* the

> >> dependent branches before running --update. You may also encounter some
> >> bugs with dependent branches (my recollection is git rebase mostly works,
> >> but sometimes you need --onto as well? The tool currently doesn't pass
> >> --onto.)

> >> - The tool requires that all commits in the branch be squashed into
> >> one.
> >> - The tool assumes if you have a tracking branch set. git cl assumes

> >> that you're tracking origin/master by default if you have nothing set; if
> >> that's the case for you, you can set the upstream branch by running git
> >> branch --set-upstream-to origin/master

> >> - There's a known bug with the tool where some names don't get

> >> updated. This has to do with the fact that 'bool' is a macro (!!!). We'll
> >> try to fix this, but until it's fixed, please do inspect the merged result
> >> with some caution.
> >>

> >> *Setup instructions:*

> >> If you don't have tools/blink_rename_merge_helper/run.py in your
> >> repository, you can stage a copy by running:
> >> # Note that this intentionally saves it into run2.py to avoid conflicting
> >> with the checked in copy of run.py.
> >> git fetch origin
> >> git show cff45fb322cbdf2badb6ededde0106161bbfb13e >
> >> tools/blink_rename_merge_helper/run2.py
> >>
> >> Then use run2.py in the instructions below, rather than run.py.
> >>

> >> *Updating branches:*

> >>> <https://chromium.googlesource.com/chromium/src/+/1c4d759e44259650dfb2c426a7f997d2d0bc73dc%5E%21/third_party/WebKit/Source/platform/scheduler/base/task_queue_selector_unittest.cc>).

> >>> I guess that wasn't expected? Should we just fix these up manually?
> >>>
> >>> - Sami
> >>>
> >>> ma 10. huhtik. 2017 klo 6.20 Emil A Eklund <e...@chromium.org> kirjoitti:
> >>>
> >>> Awesome, thank you for working on this!
> >>>
> >>> --
> >>> You received this message because you are subscribed to the Google
> >>> Groups "blink-dev" group.
> >>> To unsubscribe from this group and stop receiving emails from it, send
> >>> an email to blink-dev+...@chromium.org.
> >>>
> >>>
> >>>
> >>>
>

> --
> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev
> ---

> You received this message because you are subscribed to the Google Groups

> "Chromium-dev" group.

> To unsubscribe from this group and stop receiving emails from it, send an

> email to chromium-dev...@chromium.org.
> To view this discussion on the web visit
> https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAARdPYexKmFSFpmawLt7BmKCWb8F-6w07pxbms%3DHeFrGqWCGXA%40mail.gmail.com.

[Daniel Cheng]

Hi Mounir,

Thanks for the feedback. I think it's clear that:

1) there was a large cost to not publicly pushing the rebase helper immediately, despite the bugs

2) it wasn't clear to developers that they should reach out in case of rebase issues (as the tool was available, but not publicly announced)

I'm currently working on a writeup of lessons learned from executing this change, as well as mitigations to reduce the likelihood of blocking bugs being discovered at the last moment.

Daniel