search

machawk1 / ArchiveThumbnails

An implementation for Ahmed AlSum's "Thumbnail Summarization Techniques for Web Archives"
GNU General Public License v2.0
7 stars 5 forks source link

Docker image name difference in the build and run commands #112

Open ibnesayeed opened 7 years ago

ibnesayeed commented 7 years ago

The README file used the name archthumb when building the image while uses alsummarization at run time. Both should be the same in the documentation. Perhaps other branches of the code have the same issue.

ibnesayeed commented 7 years ago

While we are at it, the new CLI syntax uses docker image build in place of docker build and docker container run in place of docker run. We might want to accommodate this new change in the documentation as well.

machawk1 commented 7 years ago

@ibnesayeed You are correct, there is this inconsistency. Fixing it only requires adjustments to the README but the error has been propagated across other branches.

Since you are familiar with the new CLI syntax, can you provide a PR to fix the docker instructions?

ibnesayeed commented 7 years ago

Why are we not using node's official docker image and installing it manually in the ubuntu base image?

machawk1 commented 7 years ago

@ibnesayeed That, too, would be a potential optimization.

ibnesayeed commented 7 years ago

Although, I did an overhaul of the Dockerfile to make it more readable, maintainable, and cacheable. The later should yield in a faster build (especially rebuilds). For now, I did not make it inherit from node (but I did fix the version of ubuntu image) because that might cause some issues which I can't easily test as the working code is in a different branch than the master. I am creating a PR that can go in the master branch which you can cherry-pick in other branches as well, if you want. We can later modify it to inherit from node after testing.

ibnesayeed commented 7 years ago

Oops, I didn't realize that I was not working on a fork, but the main repo directly. So my commits were directly pushed to the master branch. Following are the relevant commits of which the last two are about the Dockerfile while other two are for the documentation.