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(request-node): remove dockerfile #767

Merged
merged 5 commits into from Feb 14, 2022

Conversation

alexandre-abrioux
Copy link
Member

Description of the changes

This PR is a proposal to remove the Dockerfile from the request-node package as it could bring some confusion (explanation bellow).

1/ Prod Releases

The Dockerfile is not used for production releases anymore since this responsibility was migrated to the RequestNetwork/docker-images repository.

2/ Pre Releases

Pre-production releases are now also handled by the RequestNetwork/docker-images repository.

3/ Dev Builds

The Dockerfile was still useful for development builds.

The main issue is that, when internal dependencies are modified locally, for instance @requestnetwork/ethereum-storage, the line RUN npm install would still pick the latest release of those packages on NPM instead of locally changed ones.

That is why a Dockerfile.dev now exists in the package to take care of this use case.

Overall it looks to me like this Dockerfile is not needed anymore and can bring some confusion to new contributors.

@coveralls
Copy link

coveralls commented Feb 9, 2022

Coverage Status

Coverage remained the same at 89.449% when pulling 139ad38 on request-node-dockerfile into f540b1a on master.

@alexandre-abrioux alexandre-abrioux enabled auto-merge (squash) February 14, 2022 15:27
@alexandre-abrioux alexandre-abrioux merged commit c047df3 into master Feb 14, 2022
@alexandre-abrioux alexandre-abrioux deleted the request-node-dockerfile branch February 14, 2022 15:35
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.

None yet

4 participants