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

chore: use npm build for hew #8845

Merged
merged 1 commit into from
Feb 16, 2024
Merged

chore: use npm build for hew #8845

merged 1 commit into from
Feb 16, 2024

Conversation

ashtonG
Copy link
Contributor

@ashtonG ashtonG commented Feb 14, 2024

Description

This uses the npm build for hew. previously, we used the github build, which required us to grab hew's dependencies and build on install. this should improve build times as the package is built ahead of time.

Test Plan

no testing needed -- build should succeed

Commentary (optional)

hew was already a preexisting package in npm, so this is published under the @hpe.com scope. Because of that, the package is aliased back to hew to make the pr smaller. we can look at switching to @hpe.com/hew later.

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

no ticket

This uses the npm build for hew. previously, we used the github build,
which required us to grab hew's dependencies and build on install. this
should improve build times as the package is built ahead of time.
@ashtonG ashtonG requested a review from a team as a code owner February 14, 2024 18:22
@cla-bot cla-bot bot added the cla-signed label Feb 14, 2024
Copy link

netlify bot commented Feb 14, 2024

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit df6046f
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/65cd04cec924730008806eb0
😎 Deploy Preview https://deploy-preview-8845--determined-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Feb 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ba03375) 47.58% compared to head (df6046f) 42.67%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8845      +/-   ##
==========================================
- Coverage   47.58%   42.67%   -4.91%     
==========================================
  Files        1068      751     -317     
  Lines      170265   131415   -38850     
  Branches     2240     2238       -2     
==========================================
- Hits        81016    56086   -24930     
+ Misses      89091    75171   -13920     
  Partials      158      158              
Flag Coverage Δ
harness ?
web 42.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 317 files with indirect coverage changes

@ashtonG
Copy link
Contributor Author

ashtonG commented Feb 14, 2024

observed effect: restoring cache + getting dependencies was about 9s + 45s, now up/down to 12s + 25s.

@ashtonG ashtonG merged commit c656aac into main Feb 16, 2024
77 of 89 checks passed
@ashtonG ashtonG deleted the chore/hew-npm-move branch February 16, 2024 19:02
carolinaecalderon pushed a commit that referenced this pull request Feb 19, 2024
This uses the npm build for hew. previously, we used the github build,
which required us to grab hew's dependencies and build on install. this
should improve build times as the package is built ahead of time.
maxrussell pushed a commit that referenced this pull request Mar 21, 2024
This uses the npm build for hew. previously, we used the github build,
which required us to grab hew's dependencies and build on install. this
should improve build times as the package is built ahead of time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants