LBRYCommunity/lbry-docker

Fork 0

MAINTAINER is deprecated in Dockerfile. COPY is preferred over ADD in… #20

Merged

shubhambhattar merged 2 commits from master into master

2018-10-16 08:45:59 +02:00

shubhambhattar commented

2018-10-14 14:31:01 +02:00

(Migrated from github.com)

… Dockerfile if additional functonality of ADD is not being used. Improved Shebang lines in many files.

skhameneh (Migrated from github.com) reviewed 2018-10-14 14:31:01 +02:00

alyssaoc commented

2018-10-15 22:58:57 +02:00

(Migrated from github.com)

Thanks for checking out LBRY and the contribution! If you haven't already, please make sure to check out our contributing guide.

Can we send you LBC in appreciation for your work?

Thanks for checking out LBRY and the contribution! If you haven't already, please make sure to check out [our contributing guide](https://lbry.tech/contribute). Can we send you LBC in appreciation for your [work?](https://lbry.io/faq/appreciation)

shubhambhattar commented

2018-10-15 23:28:19 +02:00

(Migrated from github.com)

Hi @alyssaoc! I appreciate that you find my contribution helpful and want to send me LBC for the same, but those LBC might be wasted on me because I don't use it.

Leopere commented

2018-10-16 08:34:19 +02:00

(Migrated from github.com)

Also thanks for reminding me that its COPY that I should be using and having brushed up on the Differences I'd realised that I could do away with wget/curl in a lot of images now.

The spee.ch container is a great catch too! I haven't touched that since I started working on this repo and started reworking my online portfolio. Are you in the Slack/Discord? Feel free to contact me if you're interested in contributing to this more.

@shubhambhattar the shebangs in an environment as specific as a container like this where nothing is going to change could go either way, I'm fine with this methodology. Is there a bigger explanation behind them as they are? I'm curious about your take on why you'd prefer this method. Also thanks for reminding me that its COPY that I should be using and having brushed up on the Differences I'd realised that I could do away with wget/curl in a lot of images now. The spee.ch container is a great catch too! I haven't touched that since I started working on this repo and started reworking my online portfolio. Are you in the Slack/Discord? Feel free to contact me if you're interested in contributing to this more.

Leopere (Migrated from github.com) approved these changes 2018-10-16 08:45:16 +02:00

Leopere (Migrated from github.com) left a comment

I'm reconsidering even really adding a personal MAINTAINER label when this goes 1.0 anyways its an artifact of me developing this stack on my personal repo for the first while.

Thanks for reminding me to look into the difference between ADD / COPY there's a lot to keep track of when containerizing an entire app stack.

Not sure if there's a huge benefit for the shebangs but if its good form I'm definitely interested in some added perspective.

I'm reconsidering even really adding a personal MAINTAINER label when this goes 1.0 anyways its an artifact of me developing this stack on my personal repo for the first while. Thanks for reminding me to look into the difference between ADD / COPY there's a lot to keep track of when containerizing an entire app stack. Not sure if there's a huge benefit for the shebangs but if its good form I'm definitely interested in some added perspective.

shubhambhattar commented

2018-10-16 21:23:48 +02:00

(Migrated from github.com)

@Leopere The reason for shebangs is because I have made it a habit of using env to find and invoke the correct interpretor (bash). You are right that in a container where you know exactly the location of bash, this is not necessary. I thought maybe if you someday change the base image in Dockerfile to something that doesn't store bash in /bin/bash, this might be helpful.

Regarding changing a lot of wget/curl commands to ADD in Dockerfile: I thought of doing it too, but all I found was I can use ADD to specify the source of url and destination folder where the file needs to be stored. The problem is there are a lot of commands that download a zip file -> unzip it -> removing the original zip file in these Dockerfiles but ADD doesn't extract zip files. So, I didn't see much advantage getting rid of the wget/curl command and using ADD in their place.

@Leopere The reason for shebangs is because I have made it a habit of using `env` to find and invoke the correct interpretor (`bash`). You are right that in a container where you know exactly the location of `bash`, this is not necessary. I thought maybe if you someday change the base image in Dockerfile to something that doesn't store `bash` in `/bin/bash`, this might be helpful. Regarding changing a lot of `wget/curl` commands to `ADD` in Dockerfile: I thought of doing it too, but all I found was I can use `ADD` to specify the source of url and destination folder where the file needs to be stored. The problem is there are a lot of commands that `download a zip file -> unzip it -> removing the original zip file` in these Dockerfiles but `ADD` doesn't extract `zip` files. So, I didn't see much advantage getting rid of the `wget/curl` command and using `ADD` in their place.

Leopere commented

2018-10-17 10:24:33 +02:00

(Migrated from github.com)

Thanks for the response @shubhambhattar well I certainly appreciate the contribution. I ended up switching out a few wget/curl's but then I also just added another build step. This does make the docker-compose version minimum be at version 3.4 but I think this is acceptable. I may end up switching out to a different container base but it's likely to be Alpine if anything as I'd like to keep things close to minimalist standards.

Thanks for the response @shubhambhattar well I certainly appreciate the contribution. I ended up switching out a few `wget/curl's` but then I also just added another build step. This does make the docker-compose version minimum be at version 3.4 but I think this is acceptable. I may end up switching out to a different container base but it's likely to be Alpine if anything as I'd like to keep things close to minimalist standards.

No reviewers

skhameneh

Leopere

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-docker#20

No description provided.