Live data from Hacker News

Fixing a bug in Google Chrome as a first-time contributor

cprimozic.net

1–10 of 170 posts

Re: Fixing a bug in Google Chrome as a first-time contributor

#2
> Although the worklet was running on a worker thread, it didn't have a WorkerGlobalScope - it had a WorkletGlobalScope.

It took me a while to see these were different, I thought it was a wrong copy-paste.

Naming things is hard, but this is a bad convention. Always put the changing bits at the beginning preferably, or the end otherwise, but never in the middle, especially when it's subtle in a rather verbose name.

Re: Fixing a bug in Google Chrome as a first-time contributor

#3

> Although the worklet was running on a worker thread, it didn't have a WorkerGlobalScope - it had a WorkletGlobalScope. It took me a while to see these were different, I thought it was a wrong copy-paste. Naming things is hard, but this is a bad convention. Always put the changing bits at the beginning preferably, or the end otherwise, but never in the middle, especially when it's subtle in a rather verbose name.

[dead]

Re: Fixing a bug in Google Chrome as a first-time contributor

#4

> Although the worklet was running on a worker thread, it didn't have a WorkerGlobalScope - it had a WorkletGlobalScope. It took me a while to see these were different, I thought it was a wrong copy-paste. Naming things is hard, but this is a bad convention. Always put the changing bits at the beginning preferably, or the end otherwise, but never in the middle, especially when it's subtle in a rather verbose name.

The changing bit is at the beginning, unless I misunderstand you.

Worker and Worklet are primitives, you can't really split them up. You can't have a LetWorkGlobalScope and an ErWorkGlobalScope, so WorkerGlobalScope and WorkletGlobalScope is the best you can do.

That said, I usually prefer the changing bit at the end. So something like GlobalScopeForWorker, GlobalScopeForWorklet. But then that's clunky, so we're back at WorkerGlobalScope and WorkletGlobalScope again.

Re: Fixing a bug in Google Chrome as a first-time contributor

#5
post #4

> Although the worklet was running on a worker thread, it didn't have a WorkerGlobalScope - it had a WorkletGlobalScope. It took me a while to see these were different, I thought it was a wrong copy-paste. Naming things is hard, but this is a bad convention. Always put the changing bits at the beginning preferably, or the end otherwise, but never in the middle, especially when it's subtle in a rather verbose name.

The changing bit is at the beginning, unless I misunderstand you. Worker and Worklet are primitives, you can't really split them up. You can't have a LetWorkGlobalScope and an ErWorkGlobalScope, so WorkerGlobalScope and WorkletGlobalScope is the best you can do. That said, I usually prefer the changing bit at the end. So something like GlobalScopeForWorker, GlobalScopeForWorklet. But then that's clunky, so we're back…

I think they mean that the change is at the end of the first word in the Variable. i.e, Worker vs. Worklet instead of Worker vs. TinyWorker?

Doesn't make too much sense to me, but I think that's what they are saying.

Re: Fixing a bug in Google Chrome as a first-time contributor

#6
post #5
post #4

Earlier quoted context omitted.

The changing bit is at the beginning, unless I misunderstand you. Worker and Worklet are primitives, you can't really split them up. You can't have a LetWorkGlobalScope and an ErWorkGlobalScope, so WorkerGlobalScope and WorkletGlobalScope is the best you can do. That said, I usually prefer the changing bit at the end. So something like GlobalScopeForWorker, GlobalScopeForWorklet. But then that's clunky, so we're back…

I think they mean that the change is at the end of the first word in the Variable. i.e, Worker vs. Worklet instead of Worker vs. TinyWorker? Doesn't make too much sense to me, but I think that's what they are saying.

That seems reasonable. I understand Worker and Worklet are established concepts in the domain, though, so better to use those names than invent new terminology.

Re: Fixing a bug in Google Chrome as a first-time contributor

#7
post #4

> Although the worklet was running on a worker thread, it didn't have a WorkerGlobalScope - it had a WorkletGlobalScope. It took me a while to see these were different, I thought it was a wrong copy-paste. Naming things is hard, but this is a bad convention. Always put the changing bits at the beginning preferably, or the end otherwise, but never in the middle, especially when it's subtle in a rather verbose name.

The changing bit is at the beginning, unless I misunderstand you. Worker and Worklet are primitives, you can't really split them up. You can't have a LetWorkGlobalScope and an ErWorkGlobalScope, so WorkerGlobalScope and WorkletGlobalScope is the best you can do. That said, I usually prefer the changing bit at the end. So something like GlobalScopeForWorker, GlobalScopeForWorklet. But then that's clunky, so we're back…

I think subconsciously I look to the end to find differences too. So maybe I'd prefer er/let to come last

Re: Fixing a bug in Google Chrome as a first-time contributor

#8
post #4

> Although the worklet was running on a worker thread, it didn't have a WorkerGlobalScope - it had a WorkletGlobalScope. It took me a while to see these were different, I thought it was a wrong copy-paste. Naming things is hard, but this is a bad convention. Always put the changing bits at the beginning preferably, or the end otherwise, but never in the middle, especially when it's subtle in a rather verbose name.

The changing bit is at the beginning, unless I misunderstand you. Worker and Worklet are primitives, you can't really split them up. You can't have a LetWorkGlobalScope and an ErWorkGlobalScope, so WorkerGlobalScope and WorkletGlobalScope is the best you can do. That said, I usually prefer the changing bit at the end. So something like GlobalScopeForWorker, GlobalScopeForWorklet. But then that's clunky, so we're back…

> That said, I usually prefer the changing bit at the end. So something like GlobalScopeForWorker, GlobalScopeForWorklet. But then that's clunky, so we're back at WorkerGlobalScope and WorkletGlobalScope again.

I wouldn't necessarily call "GlobalScopeForWorker" more clunky than "WorkerGlobalScope", just a bit longer, but also more descriptive.

Using the languages namespacing features might also make it more obvious, e.g. "Worker::GlobalScope" and "Worklet::GlobalScope" or the inverted version "GlobalScope::{Worker,Worklet}".

Looking at it from a functional programming perspective, I also like approaches of the form "GlobalScopeFor({Worker,Worklet})", i.e. a function returning the respective thing.

Naming things is hard, but the possibilities are endless...

Re: Fixing a bug in Google Chrome as a first-time contributor

#9
post #4

> Although the worklet was running on a worker thread, it didn't have a WorkerGlobalScope - it had a WorkletGlobalScope. It took me a while to see these were different, I thought it was a wrong copy-paste. Naming things is hard, but this is a bad convention. Always put the changing bits at the beginning preferably, or the end otherwise, but never in the middle, especially when it's subtle in a rather verbose name.

The changing bit is at the beginning, unless I misunderstand you. Worker and Worklet are primitives, you can't really split them up. You can't have a LetWorkGlobalScope and an ErWorkGlobalScope, so WorkerGlobalScope and WorkletGlobalScope is the best you can do. That said, I usually prefer the changing bit at the end. So something like GlobalScopeForWorker, GlobalScopeForWorklet. But then that's clunky, so we're back…

It seems to me that the problem was naming something Worklet when another thing called Worker already exists. I personally strive for unique class names when possible.

But that decision was made long before OP started contributing to this project.

Re: Fixing a bug in Google Chrome as a first-time contributor

#10
Congratulations! And thank you for the great write-up. I work with the Chromium code base a lot, and it can indeed be daunting. I use Sublime Text, which treats the code as plain text, apart from syntax highlighting. But it's also possible with at least VS Code to get some more intelligence, such as going to the definition or declaration of a function, etc.

People who have now become interested in creating their own Chromium-based browser may want to take a look at my article: https://omaha-consulting.com/how-to-fork-chromium. It gives a high-level view of what goes into maintaining a Chromium fork.

Post reply on HN