-
Notifications
You must be signed in to change notification settings - Fork 285
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
build(tools/docker/indy-sdk-cli): fix deprecation deb.nodesource.com/setup_16.x #2706
build(tools/docker/indy-sdk-cli): fix deprecation deb.nodesource.com/setup_16.x #2706
Conversation
e04d4ef
to
0beced3
Compare
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
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.
@urvish667 LGTM on the diff but please make sure your commit message is the same as your PR description + title, e.g. the git commit message should be what I pasted below.
(Note that I renamed the PR title to match the issue title that you've been working on fixing)
If you need any help with git, just let me know (and invite me as a collaborator to your fork)
build(tools/docker/indy-sdk-cli): fix deprecation deb.nodesource.com/setup_16.x
**Change Description:**
In the Dockerfile, the issue related to NodeJS was addressed by updating the NodeJS installation method to use the new NodeSource installation method. The previous method using `curl -sL https://deb.nodesource.com/setup_16.x | bash -` had become deprecated, so the new method was applied for compatibility with the latest practices.
The specific changes made in the Dockerfile are as follows:
1. Removed the old installation method for NodeJS:
```Dockerfile
# Removed the deprecated NodeJS installation method
# RUN curl -sL https://deb.nodesource.com/setup_16.x | bash -
-
Added the new NodeSource installation method:
# Added the new NodeSource installation method for NodeJS RUN curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key | gpg --dearmor -o /usr/share/keyrings/nodesource-archive-keyring.gpg \ && echo "deb [arch=amd64 signed-by=/usr/share/keyrings/nodesource-archive-keyring.gpg] https://deb.nodesource.com/node bionic main" | tee /etc/apt/sources.list.d/nodesource.list
Fixes #2701
LGTM |
@urvish667 Just checking in, are you still working on this one? |
@petermetz |
@urvish667 No worries! Well, you haven't done anything wrong, because the contributing guide does not call out explicitly that the commit message and the PR description should match (which is an error my part).
Not yet, but I can do it very quickly if you invite me as a collaborator to your cacti fork (it will give me permission to force push changes onto your feature branch that is backing your PR) |
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.
See my comment above^^
Thanks |
e340fe7
to
ed965f6
Compare
…setup_16.x **Primary Change:** In the Dockerfile, the issue related to NodeJS was addressed by updating the NodeJS installation method to use the new NodeSource installation method. The previous method using `curl -sL https://deb.nodesource.com/setup_16.x | bash -` had become deprecated, so the new method was applied for compatibility with the latest practices. The specific changes made in the Dockerfile are as follows: 1. Removed the old installation method for NodeJS: ```Dockerfile # Removed the deprecated NodeJS installation method # RUN curl -sL https://deb.nodesource.com/setup_16.x | bash - ``` 2. Added the new NodeSource installation method: ```Dockerfile RUN curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key | \ gpg --dearmor -o /usr/share/keyrings/nodesource-archive-keyring.gpg \ && echo "deb [arch=amd64 signed-by=/usr/share/keyrings/nodesource-archive-keyring.gpg] \ https://deb.nodesource.com/node bionic main" | \ tee /etc/apt/sources.list.d/nodesource.list ``` [skip ci] Co-authored-by: Peter Somogyvari <[email protected]> Signed-off-by: urvish667 <[email protected]> Signed-off-by: Peter Somogyvari <[email protected]>
ed965f6
to
4cf4cbb
Compare
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.
@urvish667 I updated your commit message, we are good to go.
Thanks @petermetz |
Change Description:
In the Dockerfile, the issue related to NodeJS was addressed by updating the NodeJS installation method to use the new NodeSource installation method. The previous method using
curl -sL https://deb.nodesource.com/setup_16.x | bash -
had become deprecated, so the new method was applied for compatibility with the latest practices.The specific changes made in the Dockerfile are as follows:
Removed the old installation method for NodeJS:
Added the new NodeSource installation method:
Fixes #2701