-
Notifications
You must be signed in to change notification settings - Fork 13
OCPBUGS-74424: migrate yarn 1 to npm #161
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
base: main
Are you sure you want to change the base?
Conversation
|
@upalatucci: This pull request references Jira Issue OCPBUGS-74424, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@upalatucci: This pull request references Jira Issue OCPBUGS-74424, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Dockerfile
Outdated
| # Run install as supper tux | ||
| USER 0 | ||
| RUN yarn install --frozen-lockfile --network-timeout 600000 && yarn build | ||
| RUN npm ci && npm run build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we need to utilize something similar to -network-timeout 600000?
RUN npm config set fetch-retries 5 && \
npm config set fetch-retry-mintimeout 20000 && \
npm config set fetch-retry-maxtimeout 120000 && \
npm ci && \
npm run build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This flag was added because of some issues during cross-platform builds (like S390x build into a AMD platform with buildx) but i'm not sure if we'll get the same issue with npm. We could skip this and then add it if we get same issues with npm. What do u think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
|
/retest |
e83c794 to
ea2579b
Compare
|
/retest |
|
/lgtm |
5b4245d to
bc486f5
Compare
d3af98e to
1c85815
Compare
galkremer1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: galkremer1, upalatucci The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1c85815 to
fa077ff
Compare
|
New changes are detected. LGTM label has been removed. |
fa077ff to
8f44169
Compare
|
@upalatucci: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
For konflux we are required to switch
I took the chance to make sure the
start-consolescript supportzshand switched fromts-loadertoesbuild-loaderfor faster loading timeRef: https://github.com/kubevirt-ui/kubevirt-plugin/pull/2708/changes#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519
kubevirt-ui/kubevirt-plugin#3298