Skip to content
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

Fix issue #3375 - Return most recent image from the cache #3417

Merged
merged 2 commits into from Jan 8, 2014
Merged

Fix issue #3375 - Return most recent image from the cache #3417

merged 2 commits into from Jan 8, 2014

Conversation

Sjord
Copy link
Contributor

@Sjord Sjord commented Jan 1, 2014

ImageGetCached searches for an image from the cache. Instead of returning the
first image it finds, it should return the most recently created image. When a
build with --no-cache then adds a new image with the same parameters, it is
used instead of the old, existing image.

@tianon
Copy link
Member

tianon commented Jan 1, 2014

Great big +1s! Nicely done. /cc @crosbymichael @creack

@tianon
Copy link
Member

tianon commented Jan 2, 2014

ping @crosbymichael @creack - this one is super small ;)

@crosbymichael
Copy link
Contributor

@Sjord Can you add a test to verify that the correct image is returned when more than one match?

@creack
Copy link
Contributor

creack commented Jan 3, 2014

@Sjord Can you rebase? Build used to take a random image for cache (because golang maps are unordered), now it is sorted, but alphabetically, it makes the build consistent, but your solution of sorting by date is even better :)

@Sjord
Copy link
Contributor Author

Sjord commented Jan 4, 2014

I merged master into my branch, without commit 1d4b7d8.

@Sjord
Copy link
Contributor Author

Sjord commented Jan 6, 2014

I don't really know how to write a test for this.

@crosbymichael
Copy link
Contributor

@Sjord It looks like you did a merge instead of a rebase. Also you have a messy commit history ( a revert ). Can you fix and squash your commits into one please?

@crosbymichael
Copy link
Contributor

You also have a doc change that has nothing to do with the code changes. You should remove that from this PR and open a new PR for the doc spelling error.

@Sjord
Copy link
Contributor Author

Sjord commented Jan 7, 2014

That spelling change came in with the merge, I don't know what went wrong. I now rebased and force-pushed to this branch, so now only my changes are visible.

@crosbymichael
Copy link
Contributor

Thanks

@crosbymichael
Copy link
Contributor

We just started to require contributors to sign the commits following these rules: https://github.com/dotcloud/docker/blob/master/CONTRIBUTING.md#sign-your-work

Each commit in your PR must be signed in the following format:
Docker-DCO-1.0-Signed-off-by: Joe Smith <joe.smith@email.com> (github: github_handle)

Please rebase and sign each commit.

Sjoerd Langkemper and others added 2 commits January 8, 2014 09:14
This reverts commit 1d4b7d8.

Docker-DCO-1.0-Signed-off-by: Sjoerd Langkemper <sjoerd@byte.nl> (github: Sjord)
ImageGetCached searches for an image from the cache. Instead of returning the
first image it finds, it should return the most recently created image. When a
build with --no-cache then adds a new image with the same parameters, it is
used instead of the old, existing image.

Docker-DCO-1.0-Signed-off-by: Sjoerd Langkemper <sjoerd@byte.nl> (github: Sjord)
@Sjord
Copy link
Contributor Author

Sjord commented Jan 8, 2014

Ok, I added a signature to each commit message.

@crosbymichael
Copy link
Contributor

LGTM

ping @creack

@creack
Copy link
Contributor

creack commented Jan 8, 2014

LGTM

creack added a commit that referenced this pull request Jan 8, 2014
Fix issue #3375 - Return most recent image from the cache
@creack creack merged commit 3be2ea0 into moby:master Jan 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants