Skip to content

build - refactor build system for easier configuration#10718

Merged
kumavis merged 11 commits intodevelopfrom
build-sys-3
Mar 26, 2021
Merged

build - refactor build system for easier configuration#10718
kumavis merged 11 commits intodevelopfrom
build-sys-3

Conversation

@kumavis
Copy link
Copy Markdown
Member

@kumavis kumavis commented Mar 25, 2021

bringing this refactor back to life #8170
this time without the factoring/libs changes

this should cause no change to the output bundle

@kumavis kumavis requested a review from a team as a code owner March 25, 2021 10:37
@kumavis kumavis requested a review from NiranjanaBinoy March 25, 2021 10:37
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [f9e8640]
Page Load Metrics (588 ± 20 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint42675394
domContentLoaded5567535874120
load5577535884120
domInteractive5557525864120

@kumavis kumavis requested a review from Gudahtt March 25, 2021 12:03
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [195d4d6]
Page Load Metrics (641 ± 19 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint45805994
domContentLoaded5797556403919
load5807566413919
domInteractive5787556393919

ryanml
ryanml previously approved these changes Mar 26, 2021
Copy link
Copy Markdown
Contributor

@ryanml ryanml left a comment

Choose a reason for hiding this comment

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

Looks good to me! Steps I ran through to test

  1. Ran yarn setup on a fresh build from this branch
  2. Built prod by running yarn dist
  3. Confirmed after sideloading the extension that there no were no background.js or client errors, and that account creation worked fine.
  4. Ran rm -rf dist/
  5. Built dev by running yarn start
  6. Confirmed after sideloading the extension that there no were no background.js or client errors, and that account creation worked fine.
  7. Confirmed that both changes to .js. and .scss files reloaded the extension appropriately.

One nit: perhaps lets tie this to a dev issue just for tracking purposes

Co-authored-by: Mark Stacey <markjstacey@gmail.com>
@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Mar 26, 2021

thanks. what are types anyways.

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

I did confirm that after a rebase, this didn't result in any changes to the output of yarn dist. Also everything seems to work as it did before - yarn dist failures result in a non-zero exit code, yarn start is tolerant of failure, etc.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [374f8c0]
Page Load Metrics (632 ± 20 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint44726184
domContentLoaded5607446304120
load5617466324120
domInteractive5607446304120

@kumavis kumavis merged commit 715f699 into develop Mar 26, 2021
@kumavis kumavis deleted the build-sys-3 branch March 26, 2021 04:26
@github-actions github-actions bot locked and limited conversation to collaborators Mar 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants