Skip to content

Switch to pnpm#1404

Merged
akellbl4 merged 1 commit intomasterfrom
switch-to-pnpm
Jul 12, 2022
Merged

Switch to pnpm#1404
akellbl4 merged 1 commit intomasterfrom
switch-to-pnpm

Conversation

@akellbl4
Copy link
Copy Markdown
Collaborator

@akellbl4 akellbl4 commented Jul 1, 2022 •
edited
Loading

installation time node_modules size
NPM i 10.79s 451M
PNPM 2.49s 342M

It's okay that size job is failing. It's because it can't install dependencies on master without pnpm-lock.yml

@akellbl4 akellbl4 force-pushed the switch-to-pnpm branch 2 times, most recently from 36c3fea to 6e0aca7 Compare July 5, 2022 19:49
@akellbl4 akellbl4 requested a review from Mavrin July 10, 2022 21:34
@akellbl4 akellbl4 marked this pull request as ready for review July 10, 2022 21:34
@akellbl4 akellbl4 requested a review from umputun as a code owner July 10, 2022 21:34
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 10, 2022 •
edited
Loading

Codecov Report

Merging #1404 (b58fa7e) into master (fddf1e2) will increase coverage by 0.17%.
The diff coverage is 80.64%.

❗ Current head b58fa7e differs from pull request most recent head 1cab9b0. Consider uploading reports for the commit 1cab9b0 to get more accurate results

@@            Coverage Diff             @@
##           master    #1404      +/-   ##
==========================================
+ Coverage   58.00%   58.17%   +0.17%     
==========================================
  Files         131      131              
  Lines        2900     2905       +5     
  Branches      699      735      +36     
==========================================
+ Hits         1682     1690       +8     
+ Misses       1214     1211       -3     
  Partials        4        4
Impacted Files Coverage Δ
frontend/app/components/comment/comment.tsx 62.22% <ø> (-0.17%) ⬇️
frontend/app/components/select/select.tsx 92.30% <ø> (ø)
...ntend/app/components/comment-form/comment-form.tsx 51.70% <64.28%> (ø)
...pp/components/comment-form/comment-form.persist.ts 94.11% <94.11%> (ø)
frontend/app/components/root/root.tsx 0.00% <0.00%> (ø)
frontend/app/store/user/actions.ts 85.88% <0.00%> (+0.69%) ⬆️
frontend/app/store/comments/actions.ts 56.57% <0.00%> (+3.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fddf1e2...1cab9b0. Read the comment docs.

This was referenced Jul 11, 2022
Merged
Merged
Mavrin
Mavrin previously approved these changes Jul 11, 2022
View reviewed changes
Comment thread Dockerfile
RUN mkdir node_modules
RUN if [ -z "$SKIP_FRONTEND_BUILD" ] ; then \
CI=true npm ci --loglevel warn \
npm i -g pnpm && \
Copy link
Copy Markdown
Collaborator

@Mavrin Mavrin Jul 11, 2022

Choose a reason for hiding this comment

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

I think we can use https://nodejs.org/api/corepack.html for activate pnpm
corepack pnpm install

Copy link
Copy Markdown
Collaborator Author

@akellbl4 akellbl4 Jul 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, I think its too early for corepack. It was introduced in v18 and it's experimental.
But worth considering when we move towards update to node18

UPD: Oh, I'm wrong it's available in v16.

umputun
umputun previously approved these changes Jul 11, 2022
View reviewed changes
@akellbl4 akellbl4 dismissed stale reviews from umputun and Mavrin via 5b3c4d1 July 11, 2022 23:28
@akellbl4 akellbl4 mentioned this pull request Jul 11, 2022
Open
@akellbl4 akellbl4 merged commit 2e777ea into master Jul 12, 2022
@akellbl4 akellbl4 deleted the switch-to-pnpm branch July 12, 2022 03:13
@paskal paskal added this to the v1.10.2 milestone Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@umputun umputun umputun approved these changes

@Mavrin Mavrin Mavrin left review comments

Assignees

No one assigned

Labels

frontend

Projects

None yet

Milestone

v1.11.0

Development

Successfully merging this pull request may close these issues.

4 participants

@akellbl4 @umputun @Mavrin @paskal