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: Move yarn to pnpm #2604

Draft
wants to merge 352 commits into
base: master
Choose a base branch
from
Draft

chore: Move yarn to pnpm #2604

wants to merge 352 commits into from

Conversation

FangSen9000
Copy link
Contributor

Please answer these questions before submitting a pull request, or your PR will get closed.

Why submit this pull request?

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What changes will this PR take into?

Objective: the purpose of this PR is to introduce advanced and popular pnpm package management tools and use pnpm to download dependency packages faster.

Effect: pnpm will be the next generation package management tool of dashboard. Migrate the package manager from the original yarn to pnpm to reduce dependency installation and compilation time.

Changes: package.json, pnpm-lock.yaml, related GitHub workflow files. The relevant documents have been updated, solutions have been written for possible problems in use, and the workflow is running normally.

The related PR

#2459

Checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@@ -49,7 +49,7 @@ jobs:
run: |
export VERSION=${{ steps.branch_env.outputs.version }}
sudo gem install --no-document fpm
git clone https://github.com/api7/apisix-build-tools.git
git clone https://github.com/FangSen9000/apisix-build-tools.git
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that we should let the API7 community merge my changes to the build script before merging this pr.
api7/apisix-build-tools#232
(if I keep it as it is, the original warehouse that has not been changed will only report an error. In order to prove that my changes are effective, I link to my modified build script warehouse.)

Copy link
Contributor Author

@FangSen9000 FangSen9000 Aug 25, 2022

Choose a reason for hiding this comment

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

BZP told me that the build tool can be changed after the dashboard is merged.
I think we can also do this. After all, it only affects the detection of RPM packaging workflow.

@@ -20,7 +20,7 @@ ARG APISIX_DASHBOARD_VERSION=master

RUN set -x \
&& apk add --no-cache --virtual .builddeps git \
&& git clone https://github.com/apache/apisix-dashboard.git -b ${APISIX_DASHBOARD_VERSION} /usr/local/apisix-dashboard \
&& git clone https://github.com/FangSen9000/apisix-dashboard.git -b ${APISIX_DASHBOARD_VERSION} /usr/local/apisix-dashboard \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I keep it as it is, the original warehouse that has not been changed will only report an error. In order to prove that my change is effective, I link to my changed warehouse. Just like RPM workflow. Before merging, please remind me to change it back.

@FangSen9000 FangSen9000 changed the title chore: Move yarn to pnpm chore: Move yarn to pnpm ( Finished ) Aug 24, 2022
@FangSen9000
Copy link
Contributor Author

FangSen9000 commented Aug 24, 2022

Although the previous work has been completed, @SkyeYoung soon merged a new pr. This is a big change, and it conflicts with me. (https://github.com/apache/apisix-dashboard/pull/2583/files)

Therefore, based on the latest and modified master, I relocated the dashboard from yarn to pnpm.

In fact, the last work was completed seven days ago, and has used pnpm on the dashboard for two weeks. I am waiting for @bzp2010 's comments on the construction tool (the workflow tool script is stored in other Repository.) @Baoyuantop @guoqqqi

@FangSen9000 FangSen9000 marked this pull request as ready for review August 25, 2022 01:07
cache: 'yarn'
cache-dependency-path: web/yarn.lock

- name: Install PNPM
Copy link
Member

Choose a reason for hiding this comment

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

Why not use the officially provided
https://pnpm.io/continuous-integration#github-actions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"When downloading the relevant environment, it seems that there are some restrictions. Maybe it can be solved by storing it in the warehouse through some link, but I don‘t find it. This picture is the reason why the three people are useless. Action is prohibited. It clearly meets the requirements he said, but it can't be used."`
1661525327899
1661525339301

@guoqqqi You can look at the pictures. If anyone can solve it, it's best.

Copy link
Member

Choose a reason for hiding this comment

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

OK, it seems to be due to some permissions of the Apache repository.🤣

cache-dependency-path: web/yarn.lock

- name: Install PNPM
run: npm i -g [email protected]
Copy link
Member

Choose a reason for hiding this comment

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

ditto

cache-dependency-path: web/yarn.lock

- name: Install PNPM
run: npm i -g [email protected]
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@guoqqqi
Copy link
Member

guoqqqi commented Aug 26, 2022

Please check for failed E2E tests and try to fix them, if there is a failed CI then it will not be possible to agree to merge it

@FangSen9000
Copy link
Contributor Author

Please check for failed E2E tests and try to fix them, if there is a failed CI then it will not be possible to agree to merge it

I will try to refresh again and again until I get the PR exactly right.

In addition, you can see that I seem to find the reason why CI can't find elements or contents. This is the case with both yarn and pnpm: the page can't be loaded, and it hasn't been waiting for a long time. The probability of its occurrence is as high as 50%. I put forward an issue with a video inside.

@guoqqqi

#2605

FangSen9000 and others added 30 commits September 27, 2022 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants