Skip to content

Add sanitized-string format#5007

Merged
julienw merged 10 commits into
firefox-devtools:mainfrom
valenting:sanitized-string
Jun 4, 2024
Merged

Add sanitized-string format#5007
julienw merged 10 commits into
firefox-devtools:mainfrom
valenting:sanitized-string

Conversation

@valenting

Copy link
Copy Markdown
Contributor

See ⚙ D211171 Bug 1898171 - Add SanitizedString profiler marker format r=acreskey,julienw

This change adds the ability to sanitize fields that are not URLs or FilePaths.
This is necessary when we want to add domains or other PII fields that are not exactly the same as URL or FilePath.

@codecov

codecov Bot commented May 22, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.49%. Comparing base (15336cc) to head (4fe78ba).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5007   +/-   ##
=======================================
  Coverage   88.48%   88.49%           
=======================================
  Files         304      304           
  Lines       27393    27398    +5     
  Branches     7400     7402    +2     
=======================================
+ Hits        24240    24245    +5     
  Misses       2931     2931           
  Partials      222      222           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@julienw julienw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have told you earlier, sorry about that.

You need to add some code to display the data as well:

You can also add one case in

it('can apply a variety of formats', function () {

After that I think it will be good to go for the frontend part! (but let's wait that the c++ part is ready, just to make sure the version numbers do not change in-between).

@julienw julienw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the added tests and types, this looks good to me now!

We can land it as soon as the gecko part is ready too. I'm gonna ping Adam to have a look at it today.

@valenting

Copy link
Copy Markdown
Contributor Author

@julienw appveyor failed because of a nvm issue.
The gecko patches are ready to land at your convenience.

@valenting valenting closed this Jun 4, 2024
@valenting valenting reopened this Jun 4, 2024
@valenting

Copy link
Copy Markdown
Contributor Author

Seems all green now.

@julienw

julienw commented Jun 4, 2024

Copy link
Copy Markdown
Contributor

appveyor isn't marked as "required" here, esp because it tends to fail sometimes like you've seen. It's more here for our information.
Landing now, thanks again!

@julienw julienw merged commit ee83710 into firefox-devtools:main Jun 4, 2024
@julienw julienw mentioned this pull request Jun 4, 2024
julienw added a commit that referenced this pull request Jun 4, 2024
[Florian Quèze] Only count markers that are part of the selected range when hovering a marker in the marker chart. (#5004)
[Valentin Gosu] Add sanitized-string format (#5007)

Thanks also to our localizers:
zh-CN: Olvcpr423
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants