Examples:https://chromium-review.googlesource.com/c/chromium/src/+/789090 removes the chrome:: namespace from some otherwise-global-scope functions. Per the above policy it would be nice to keep these in a namespace, so for example:
- In https://chromium-review.googlesource.com/c/chromium/src/+/789090/7/chrome/browser/ui/browser_navigator.h and https://chromium-review.googlesource.com/c/chromium/src/+/789090/7/chrome/browser/ui/browser_navigator_params.h , we could add namespace navigate::, and place all functions inside it, as well as NavigateParams (which could become navigate::Params to reduce redundancy).
- In https://chromium-review.googlesource.com/c/chromium/src/+/789090/7/chrome/browser/ui/browser_navigator_params.h , FillNavigateParamsFromOpenURLParams() could further be moved inside NavigateParams (or navigate::Params) as a static member FillFromOpenURLParams(). Better yet, make it a nonstatic member and eliminate the NavigateParams* argument.
- In https://chromium-review.googlesource.com/c/chromium/src/+/789090/7/chrome/browser/ui/singleton_tabs.h , we could add namespace singleton_tab::, thus making the functions here singleton_tab::Show(), singleton_tab::ShowRespectRef(), singleton_tab::GetIndex(), etc.
--
--
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 view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAAHOzFA1Woi1P%3Di-%2Bt1HKRecY8Gouv0AzdpLKFLhu3JLejY7BA%40mail.gmail.com.
I sympathize in spirit, but I don't think it's practical to have a meaningful change in this area. I tried to enforce better namespace usage when the codebase and team were much smaller, and failed. I think it's too hard to enforce across such a large team unless we had a super rigid definition that can be automatically checked of what every namespace should be. Changing the existing usage is a herculean task with relatively little upside.
In the end, I think people should give good names to things. Namespaces are one way to do that but not the only way. If people give good names to things it doesn't really matter exactly how, and if people give bad names to things namespaces won't help.
If we want to make some progress, I would suggest finding enums in the global namespace (or maybe the common big ones like base and content) and changing them to well-named enum classes. There are some dubious ones so the benefit is greater, and it's not super hard. For example, I ran into an enum value of "TAB" in the global namespace today:
If I understand correctly, a major argument in 2013 for dropping the chrome:: namespace is that there was no way to fix existing usage.
I'm just saying this because no-one's explicitly mentioned it, but a major issue with the global namespace is that all those symbols show up in every other namespace. This can lead to surprising behaviour when header files are missing.
I don't know what the last sentence refers to.
--
--
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 view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAHixhFpF%2BQjg9%2BQGGxRtqV_6hMKHr3DEdLbEPRW0eRrBe%2BmC8g%40mail.gmail.com.
The "Jumbo builds" work has run into symbol conflicts, although I think that typically happens with file-scope symbols and not global-scope ones.
--
--
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 view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/op.zafbpxzlrbppqq%40cicero2.linkoping.osa.
I do not like the idea of introducing namespaces that neither match top-level components nor sub-component filenames. For example, navigate::Navigate(foo) doesn't help me to discover that the definition is in chrome/browser/ui/browser_navigator.cc
Given that your reply is the only one favorable to this proposal so far, and what you actually want isn't something I really support :), I'm leaning towards withdrawing this proposal, and letting Colin Blundell's description of best practices (which I think is accurate) stand; but I will wait another day or two for further debate.
--
--
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 view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAAHOzFACnvtf%3Dh_A6hvfqkK4nJxhvmci%3DCjF1-KdNaChMtZb-Q%40mail.gmail.com.
It sounds like there was at least one related best practice, then:- Prefer enum classes instead of enums in the global namespace
What about constants in top-level embedders? Chrome URL constants are currently in the chrome namespace -- if we move them to the top-level namespace, it gets harder to figure out where a constant is from (some header file? an anonymous namespace in the current file?) and adds quite a lot of top-level names.
As another example, chrome/browser/media's "kUploadUrl" was recently moved out of the chrome namespace. Predictably, this clashes with other constants as in this anonymous namespace. Is there a recommended way of ensuring constant names are specific enough to prevent this?
On Tue, Nov 28, 2017 at 12:10 PM, Peter Kasting wrote:Given that your reply is the only one favorable to this proposal so far, and what you actually want isn't something I really support :), I'm leaning towards withdrawing this proposal, and letting Colin Blundell's description of best practices (which I think is accurate) stand; but I will wait another day or two for further debate.Hearing nothing further, I withdraw this proposal.Current recommended practice (copied from Colin's earlier mail):- If you're in a top-level embedder (//chrome, //ios/chrome, ...), don't use a namespace.- Otherwise, use the namespace of the module that you're in (which is typically highly predictable, e.g., //content/<foo> -> ::content, //components/<foo>, -> ::foo, etc).
On Tue, Sep 4, 2018 at 9:32 PM Alexey Baskakov <lo...@chromium.org> wrote:On Saturday, December 2, 2017 at 7:50:38 AM UTC+11, Peter Kasting wrote:On Tue, Nov 28, 2017 at 12:10 PM, Peter Kasting wrote:Given that your reply is the only one favorable to this proposal so far, and what you actually want isn't something I really support :), I'm leaning towards withdrawing this proposal, and letting Colin Blundell's description of best practices (which I think is accurate) stand; but I will wait another day or two for further debate.Hearing nothing further, I withdraw this proposal.Current recommended practice (copied from Colin's earlier mail):- If you're in a top-level embedder (//chrome, //ios/chrome, ...), don't use a namespace.- Otherwise, use the namespace of the module that you're in (which is typically highly predictable, e.g., //content/<foo> -> ::content, //components/<foo>, -> ::foo, etc).
Can you clarify "a top-level embedder" term?Should everything under src/chrome/ be in a global namespace?For example:
1) We have src/apps/ directory.
2) Some apps-related code (which uses profile.h) lives in src/chrome/browser/apps/3) So /apps/ is a part of chrome/Should apps:: namespace be avoided in favor of global namespace?Thanks in advance for any clarifications.Your (3) seems off to me: just because some bits related to apps are in chrome/ does not mean all of apps/ is in chrome/.I don't think the rules are very clear-cut. You can probably use or not use an apps:: namespace depending on what you think is clearer. Just don't do something like chrome::apps:: for the bits in chrome/, and don't do repetitious names like apps::AppsXYZ.PK
--
--
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 view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAAHOzFCbFmjFYFZ8FDzYEnsOXCR5bCatooC8vUZs_59LAS_cFw%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CACi5S_1RCv9hv%2B6oU%2BA9chX9_ra6%3DrpwUSnkoj1EQP98YbuHoA%40mail.gmail.com.
PK
Avoiding repetitious names (like using foo::Registry instead of foo::FooRegistry) forces you to use short file names (registry.cc instead of foo_registry.cc)
As a result, we can have several registry.cc in the project.This might be a problem since most text editors and IDEs represent "Open Source File" UI as a global flat list of all files.