Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add code comments to better describe date formatting util #29242

Merged
merged 5 commits into from
Feb 19, 2025

Conversation

Prithpal-Sooriya
Copy link
Contributor

@Prithpal-Sooriya Prithpal-Sooriya commented Dec 16, 2024

Description

I'm adding these utils for mobile, and we didn't really explain in tests and code comments what each of the date formatting sections do.

This adds code comments on both tests and code to make it clear what the date formatting does.

FUTURE NOTE: It would be nice to extract this into CORE to be reused by extension and mobile. However mobile does not support Intl.DateTimeFormat

Open in GitHub Codespaces

Related issues

Fixes: N/A

Manual testing steps

N/A - we are just adding docs/code comments.

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@Prithpal-Sooriya Prithpal-Sooriya requested a review from a team as a code owner December 16, 2024 16:34
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-notifications DEPRECATED: Use "team-assets" instead label Dec 16, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [f8fda09]
Page Load Metrics (1718 ± 106 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint39521871643359172
domContentLoaded140421781694220106
load143421881718221106
domInteractive23150423014
backgroundConnect85624168
firstReactRender1693402814
getState45512147
initialActions01000
loadScripts10071665127118689
setupStore68315209
uiStartup164429002025285137
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [9754c22]
Page Load Metrics (1827 ± 66 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint36719931724321154
domContentLoaded15352299178414268
load16032305182713766
domInteractive28663694
backgroundConnect9101442412
firstReactRender1784372512
getState5481294
initialActions00000
loadScripts10951808133313263
setupStore75619189
uiStartup18552580208114469
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Copy link
Contributor

@mathieuartu mathieuartu left a comment

Choose a reason for hiding this comment

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

🏆🏆🏆

@metamaskbot
Copy link
Collaborator

Builds ready [16a6c02]
Page Load Metrics (1794 ± 97 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint39621931715366176
domContentLoaded14912155176120699
load15112181179420197
domInteractive239443199
backgroundConnect985372311
firstReactRender158234209
getState57223199
initialActions01000
loadScripts10891627130715675
setupStore65614147
uiStartup174024702044231111
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [5c2b251]
Page Load Metrics (1792 ± 99 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14612272179320699
domContentLoaded14522253175820699
load14602265179220699
domInteractive249137189
backgroundConnect983362512
firstReactRender1694532914
getState56214126
initialActions01000
loadScripts10541631130114369
setupStore689202412
uiStartup163225782168268129
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@Prithpal-Sooriya Prithpal-Sooriya added this pull request to the merge queue Dec 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 18, 2024
@Prithpal-Sooriya Prithpal-Sooriya added this pull request to the merge queue Feb 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 17, 2025
@Prithpal-Sooriya Prithpal-Sooriya added this pull request to the merge queue Feb 19, 2025
Merged via the queue into main with commit 159a416 Feb 19, 2025
77 checks passed
@Prithpal-Sooriya Prithpal-Sooriya deleted the docs/add-format-date-test-comments branch February 19, 2025 10:10
@github-actions github-actions bot locked and limited conversation to collaborators Feb 19, 2025
@metamaskbot metamaskbot added the release-12.14.0 Issue or pull request that will be included in release 12.14.0 label Feb 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.14.0 Issue or pull request that will be included in release 12.14.0 team-notifications DEPRECATED: Use "team-assets" instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants