Remove scripts #4
Conversation
| @@ -1,24 +0,0 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Please do not remove this script as it is used for testing purposes. It needs to be updated do reflect the Microsoft/dotnet docker hub.
There was a problem hiding this comment.
Dan is setting up automated build, these won't be possible to run. Also it's only 2 people who can run these commands, better keep it to yourself to put it right on the root dir of the project.
There was a problem hiding this comment.
publish.sh can go but I don't understand why build.sh wouldn't be possible to run. Can you explain?
There was a problem hiding this comment.
@MichaelSimons because there's no point of keeping build.sh. First reason is, everyone knows docker build -t. Seconds reason is, once it's on Docker Hub people will just pull the image instead of building it from this repo (also, they should not, because what's on Docker Hub is more secure than their local and outdated copy of this Dockerfile).
There was a problem hiding this comment.
@ahmetalpbalkan we are looking at this from two different view points. I am looking at it from a development/test/ci perspective while you are looking at it from a docker consumer. From a development perspective it is extremely useful as it builds all of the images with the appropriate tags so that they can be tested before pushing. Eventually we will hook up an official CI. You in essence have similar scripts in the aspnet repo. I believe public consumers won't get hung up on this script until we convert it over to CI.
There was a problem hiding this comment.
Still couldn't get your point. If the image doesn't build, Docker Hub build will show the build as failed and you won't be able to release the tag.
Having a script that people would neither understand or nor use in the root of the repo doesn't help with anything. Also that script does not even have a non-zero exit code when something in it fails (i.e. it's not strict bash mode).
There was a problem hiding this comment.
@ahmetalpbalkan as a developer, if you make a change to the Dockerfile, you would like to build the all of the images with all of the tags locally and test your changes before committing. This is where the build script becomes useful.
There was a problem hiding this comment.
Yeah that's just called "docker build ." You don't need a file for that, everyone who would contribute to this image would know this. Please write documentation instead.
There was a problem hiding this comment.
@ahmetalpbalkan please resolve conflicts so I can merge this pull request.
There was a problem hiding this comment.
I am mobile now, feel free to take over and delete the files. Closing.
# This is the 1st commit message: Fixups for merging into nightly # The commit message #2 will be skipped: # Update nightly SDK to 2.1.300-rtm-008823 (#519) # # The commit message #3 will be skipped: # Update nightly SDK to 2.1.300-rtm-008823 (#538) # # The commit message #4 will be skipped: # Remove 2.0-arm preview images (#537) # # The commit message #5 will be skipped: # Update nightly SDK to 2.1.300-rtm-008838 (#539) # # The commit message #6 will be skipped: # Update nightly SDK to 2.1.300-rtm-008866 (#542) #
# This is the 1st commit message: Move nightly images to MCR # The commit message #2 will be skipped: # Build changes # The commit message #3 will be skipped: # WIP # The commit message #4 will be skipped: # Build tweaks # The commit message #5 will be skipped: # Test updates # The commit message #6 will be skipped: # tweaks # The commit message #7 will be skipped: # Update image-builder
No need for these in the automatic build mode.