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

Technical story/build and run maintenance #40

Merged
merged 4 commits into from
Jun 12, 2023

Conversation

Riadabd
Copy link
Contributor

@Riadabd Riadabd commented Jun 12, 2023

(Addresses DL-5222) Fixes the build issue that was occuring due to nodejs/readable-stream#516 and adds a package-log.json file.

@Riadabd Riadabd requested a review from Windvis June 12, 2023 10:54
Copy link
Contributor

@Windvis Windvis 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 👍.

2 extra things I would do if you enable the lock file:

  • copy it over when building the docker image so that it uses the same dependency versions
  • use npm ci to install dependencies in the drone files. That will speed up installs a lot since it only installs what's in the lock file.

@Riadabd
Copy link
Contributor Author

Riadabd commented Jun 12, 2023

For the npm ci part, I am going to remove drone and add woodpecker. Is it possible to have a npm ci step in woodpecker? Is it in the install section of the .verify-pr.yml file?

Dockerfile Outdated Show resolved Hide resolved
@Windvis
Copy link
Contributor

Windvis commented Jun 12, 2023

For the npm ci part, I am going to remove drone and add woodpecker. Is it possible to have a npm ci step in woodpecker? Is it in the install section of the .verify-pr.yml file?

I guess so? We just need to replace npm install where needed.

@Windvis Windvis merged commit 5d971a9 into development Jun 12, 2023
@Windvis Windvis deleted the technical-story/build-and-run-maintenance branch June 12, 2023 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants