-
Notifications
You must be signed in to change notification settings - Fork 24.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Lazy init for event name mapping registry
Summary: ## Changelog: [Internal] - Noticed this when setting up C++ unit tests for WebPerformance - the change in this diff both makes sure there is no "global constructor" warning generated when `-Wglobal-constructors` is enabled, and also makes this bit potentially a little bit more efficient, as we don't pay for the registry construction if we don't use the WebPerformance API. Reviewed By: christophpurrer Differential Revision: D43663521 fbshipit-source-id: 59952f2415f49bb455a3443b3bfd8971108ac72b
- Loading branch information
1 parent
a0adf57
commit 3997fc1
Showing
1 changed file
with
47 additions
and
42 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3997fc1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rshest why aren't we using enums here; instead?
It'll allow more code reusability and make it easier to keep them up-to-date with W3 specs.
PS: Was just going through the recent commits...
PPS: I would like to help with that refactor if it's okay to do so.
3997fc1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Pranav-yadav The purpose of this table is purely to map one string to another, so I'm not sure whether using enums would improve anything in this particular case.
If you mean using internally an enum type instead of a string inside the
RawEvent
data structure - well, perhaps that could make sense indeed. But note that that would be quite a bit bigger change (and completely orthogonal to this PR) :)3997fc1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the background!
Definitely, that would be completely orthogonal... I forgot to mention the context, (general qn).
I am just trying to understand the code and why it's kept/used that way.
Meanwhile, do let me know if there are some beginner-friendly refactors/changes/feat's that I can help with :)