Issue #104 Simplenote tags. #224

Merged
merged 14 commits into from May 20, 2011

Conversation

Projects
None yet
3 participants
Contributor

darrylhthomas commented May 3, 2011

Revised pull request. I've added a method to the sync service protocol that allows services to apply metadata updates/etc even if the version comparison comes back as same. In the case of simplenote, this gives us an opportunity to migrate metadata/tags upon the first use of the new api.

Looking forward to this landing in NV & nvAlt.

Owner

scrod commented May 19, 2011

Darryl, thanks very much for your work, I really truly appreciate it. It will take me a while to work in an upgrading step and to ensure that the conversion from dates to sync-nums works properly.

Alan: You shouldn't need to worry as an upcoming version of NV will very likely make most forks redundant.

(Edit: meant to say Darryl)

Contributor

darrylhthomas commented May 19, 2011

No problem. Just to be clear: as currently implemented, the "upgrade" doesn't require an explicit step. When comparing local/remote notes, a check is made in the local sync metadata for a syncnum property. If such a property doesn't exist, then the api1-style comparison is made and as of commit c9ef643 each note is given a chance to sync metadata (tags in this case, but could also be systemtags, etc) even if the legacy comparison has deemed a note "in sync".

If you prefer this to be architected differently, just let me know, and I'll change things accordingly. Additionally, if you'd like me to amend this to include sync support for some of the stuff you're working on that the public hasn't seen yet (Markdown, etc), just let me know.

Thanks,
D

Owner

scrod commented May 19, 2011

Darryl,

Oh, don't worry about that — I don't want you to go to any extra effort on my behalf. I did notice that it ran -applyMetadataUpdatesToNote: on every note regardless of whether it actually changed. I was actually planning on adding migration logic to the upgradeDatabaseIfNecessary method for that, to avoid incurring more overhead for very long lists of notes (some users have upwards of 3000). I also still need to add an incremental list-update step, which will involve updating and adding, but not deleting.

One question, though: how would you like your specific changes to be licensed?

Contributor

darrylhthomas commented May 19, 2011

Right on. I can see where amending upgradeDatabaseIfNecessary would be beneficial. In my approach I was attempting to avoid touching the NV core as much as possible and limit my changes to Simplenote-related classes. Per your email to me from a while back, my intent was to get the feature working with minimal changes allowing us to perhaps tackle the the larger task of simplifying the sync engine, as you've hinted at in the past.

As for extra effort, that's what I'm here for, man. To the extent that my time permits, I want to be a delegate resource for you, so feel free to throw things back at me. I just joined the dev mailing list as well.

Thanks,
D

@scrod scrod added a commit that referenced this pull request May 20, 2011

@scrod scrod Merge pull request #224 from darrylhthomas/simplenote-tags
Issue #104 Simplenote tags. (Initial merge, pending database update logic)
3a45ca9

@scrod scrod merged commit 3a45ca9 into scrod:master May 20, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment