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

fixes the duplicate entries in 1.0.0 #1566

Merged
merged 2 commits into from Aug 14, 2013
Merged

fixes the duplicate entries in 1.0.0 #1566

merged 2 commits into from Aug 14, 2013

Conversation

skurfer
Copy link
Member

@skurfer skurfer commented Aug 13, 2013

This was part of another pull request that won't be merged for a while due to the sweeping changes it contains.

It fixes the bug in release and works fine with the ARC branch.

skurfer added 2 commits Aug 13, 2013
This manually does what `setIdentifier:` would do, with the exception of adding the object to `objectDictionary`.

When objects were read from the disk cache on startup, an object would appear once in the catalog for every time it appeared in one of the indexes. When scanning from the entry (instead of reading from the cache) duplicates did not appear.

fixes #1459
Instead of duplicating parts of `setIdentifier:`, allow it to take a parameter that controls its behavior so it can be used in all contexts.
@skurfer skurfer mentioned this pull request Aug 13, 2013
5 tasks
@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Aug 14, 2013

Looks good. Cheers for adding the clarification method. Looks more verbose, but definitely clearer for anybody reading the code/means we can set identifiers for objects in the future.

Looks like there are more things in #1455 that could be cherry picked, but probably not important for v1.1

pjrobertson added a commit that referenced this issue Aug 14, 2013
fixes the duplicate entries in 1.0.0
@pjrobertson pjrobertson merged commit 256c049 into release Aug 14, 2013
@pjrobertson pjrobertson deleted the dupes branch Aug 14, 2013
@skurfer
Copy link
Member Author

@skurfer skurfer commented Aug 14, 2013

Cool. This was against master, so it won't be in the current release, but we could hurry up with 1.1.1 after.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Aug 14, 2013

Oh, right. Yeah 1.1.1 can come a week or so later, if we find any glaring bugs :)

On 14 Awst 2013, at 20:44, Rob McBroom notifications@github.com wrote:

Cool. This was against master, so it won't be in the current release, but we could hurry up with 1.1.1 after.


Reply to this email directly or view it on GitHub.

@skurfer
Copy link
Member Author

@skurfer skurfer commented Aug 14, 2013

Wait, it looks like it went into release after all. Never mind.

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Aug 14, 2013

Yeah I thought I saw that :/

hehe

On 14 Awst 2013, at 20:53, Rob McBroom notifications@github.com wrote:

Wait, it looks like it went into release after all. Never mind.


Reply to this email directly or view it on GitHub.

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

2 participants