Details
- Reviewers
atul ashoat - Commits
- rCOMMc2ddd80af740: [keyserver] Revert D4422 and D4429
Ran the Docker keyserver build and since this is a reverting change, things should work as they did before.
Diff Detail
- Repository
- rCOMM Comm
- Branch
- master
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
keyserver/bash/source-nvm.sh | ||
---|---|---|
1 ↗ | (On Diff #14445) |
Done!
ShellCheck complains when you remove the shebang: Tips depend on target shell and yours is unknown. Add a shebang. Instead, I can change it to sh? But I don't think that change is necessary since this code will run in either bash or sh. |
keyserver/package.json | ||
16 ↗ | (On Diff #14445) | I think you're right, this change just reverted it to what it was before but as you can see that command fails in the keyserver directory: |
Removed executable bit from source-nvm.sh. Addressed rest of inline feedback in comments below.
keyserver/package.json | ||
---|---|---|
16 ↗ | (On Diff #14452) | It still is . /bash/source-nvm.sh |
keyserver/bash/source-nvm.sh | ||
---|---|---|
1 ↗ | (On Diff #14445) | Okay fair. I still think it would be better to replace with sh then... if it's written for sh, it's more likely to work for bash than the reverse (being written for bash and working for sh) |
keyserver/package.json | ||
16 ↗ | (On Diff #14445) |
Not sure what you mean, the left side of D4422 clearly shows a working command |
Address inline feedback:
- Replaced bash with sh
- Replaced all usages of . /bash/source-nvm.sh with . bash/source-nvm.sh. I'm not sure why this diff didn't revert the first time, I was wrong about that.
Test Plan:
- Searched for all usages of source-nvm.sh and made sure they didn't have a forward slash at the beginning/were executable.
- Ran the Docker keyserver build and it worked with no errors.
- Ran ShellCheck on source-nvm.sh.