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.

Sign up for GitHub

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

Jump to bottom

ci - create source-map-explorer build-artifacts #7141

Merged
merged 4 commits into from
Sep 11, 2019
Merged

ci - create source-map-explorer build-artifacts #7141

merged 4 commits into from
Sep 11, 2019

Conversation

kumavis
Copy link
Member

@kumavis kumavis commented Sep 10, 2019 •
edited
Loading

Adds CI script to generate mini webapps for exploring the relative size of deps in bundles based on sourcemaps.
Modifiies the metamaskbot comment to include links to these webapps with a new format

See the latest metamaskbot comment for an example in practice

@kumavis kumavis force-pushed the ci-source-map-explorer branch from 3909739 to 68092d8 Compare September 10, 2019 09:55
@metamaskbot
Copy link
Collaborator

Builds ready [68092d8]: chrome, firefox, edge, opera

@metamaskbot
Copy link
Collaborator

Builds ready [7971f26]

@Gudahtt
Copy link
Member

Gudahtt commented Sep 10, 2019

I'm having trouble making sense of these results, particularly for ui. I know for example that app/scripts/lib/util is used in the UI, but I can't find it anywhere in that page. Instead it seems to indicate that the only top-level directories used are ui and node_modules.

@kumavis
Copy link
Member Author

kumavis commented Sep 10, 2019

@Gudahtt hmm, yes. mysterious! I know that terser/gulp-uglify-es output sourcemaps fail strict validation, so they could be eaten by some sort of bug. or maybe browserify deduped them with something in gaba. not sure

@metamaskbot
Copy link
Collaborator

Builds ready [53bc905]

@kumavis
Copy link
Member Author

kumavis commented Sep 10, 2019

I'll be adding some other dependency analysis tools in separate PRs

@kumavis kumavis force-pushed the ci-source-map-explorer branch from 53bc905 to 9f3bea6 Compare September 11, 2019 14:58
@metamaskbot
Copy link
Collaborator

Builds ready [9f3bea6]

@Gudahtt
Copy link
Member

Gudahtt commented Sep 11, 2019

Cool, the missing script I pointed out earlier is present now. Maybe it was due to the terser update? ¯_(ツ)_/¯

@kumavis
Copy link
Member Author

kumavis commented Sep 11, 2019

I actually dont see it. its supposed to be in the ui bundle?

Gudahtt
Gudahtt approved these changes Sep 11, 2019
View reviewed changes
Copy link
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.

Looks good!

@Gudahtt
Copy link
Member

Gudahtt commented Sep 11, 2019

I actually dont see it. its supposed to be in the ui bundle?

Yep, it's in the bottom-right.

Though... I'm not sure what ui.js on the bottom left is 🤔
Maybe it's lines in the bundle that were introduced by the bundler? Not sure.

@kumavis
Copy link
Member Author

kumavis commented Sep 11, 2019

Yep, it's in the bottom-right.

ah, it was hidden. needed to enlarge the window. cool

@kumavis
Copy link
Member Author

kumavis commented Sep 11, 2019

yeah I still need to debug our sourcemaps and figure out why the strict sourcemap validator doesnt like them (it checks every mapping for matching var names, like 200k checks per bundle)

@kumavis kumavis merged commit 629f5ef into develop Sep 11, 2019
@kumavis kumavis deleted the ci-source-map-explorer branch September 11, 2019 15:35
@Gudahtt Gudahtt mentioned this pull request Sep 11, 2019
Merged
Gudahtt added a commit that referenced this pull request Sep 17, 2019
@Gudahtt
Merge remote-tracking branch 'origin/develop' into sync-master-with-d…
dcc38dd 
…evelop

* origin/develop: (31 commits)
  Performance: Delivery optimized images (#7176)
  Add `appName` message to each locale
  Remove the disk store (#7170)
  Update @hapi/subtext as per security advisory (#7172)
  Add fixes for German translations (#7168)
  Fix recipient field of approve screen (#7171)
  3box integration 2.0 (#6972)
  ci - metamaskbot - include links to dep-viz and all artifacts (#7155)
  Replace `undefined` selectedAddress with `null` (#7161)
  Add polyfill for AbortController (#7157)
  Remove redundant error logging (#7158)
  Set minimum Firefox version to v56.2 to support Waterfox (#7156)
  ci - install deps with "--har" flag to capture network activity (#7143)
  ci - create source-map-explorer build-artifacts (#7141)
  ci - build-artifacts - generate sesify-viz for inspecting deps (#7151)
  Publish GitHub release from master branch (#7136)
  fix rinkeby spelling (#7148)
  deps - move gulp-terser-js to devDeps
  test:integration - fix renamed test data file
  lint fix
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gudahtt Gudahtt Gudahtt approved these changes

@danjm danjm Awaiting requested review from danjm

@whymarrh whymarrh Awaiting requested review from whymarrh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kumavis @metamaskbot @Gudahtt