Merge lp://staging/~tomdroid-dev/tomdroid/sync-ui into lp://staging/~tomdroid-maintainers/tomdroid/main

Proposed by Rodja
Status: Merged
Approved by: Olivier Bilodeau
Approved revision: 241
Merged at revision: 186
Proposed branch: lp://staging/~tomdroid-dev/tomdroid/sync-ui
Merge into: lp://staging/~tomdroid-maintainers/tomdroid/main
Diff against target: 4072 lines (+3072/-290)
40 files modified
.classpath (+3/-0)
AndroidManifest.xml (+24/-9)
data/tomdroid-4.svg (+612/-0)
default.properties (+1/-1)
res/anim/pulse.xml (+10/-0)
res/drawable/syncbutton_background.xml (+16/-0)
res/drawable/syncbutton_background_focus.xml (+4/-0)
res/drawable/syncbutton_background_pressed.xml (+4/-0)
res/layout/actionbar.xml (+84/-0)
res/layout/main.xml (+10/-4)
res/layout/main_list_item.xml (+25/-6)
res/layout/note_view.xml (+32/-14)
res/menu/main.xml (+6/-8)
res/values/arrays.xml (+11/-0)
res/values/strings.xml (+13/-2)
res/xml/preferences.xml (+18/-0)
src/org/tomdroid/Note.java (+36/-6)
src/org/tomdroid/NoteManager.java (+45/-9)
src/org/tomdroid/NoteProvider.java (+5/-2)
src/org/tomdroid/sync/ServiceAuth.java (+33/-0)
src/org/tomdroid/sync/SyncManager.java (+106/-0)
src/org/tomdroid/sync/SyncService.java (+196/-0)
src/org/tomdroid/sync/sd/NoteHandler.java (+2/-1)
src/org/tomdroid/sync/sd/SdCardSyncService.java (+68/-44)
src/org/tomdroid/sync/web/AnonymousConnection.java (+62/-0)
src/org/tomdroid/sync/web/OAuthConnection.java (+278/-0)
src/org/tomdroid/sync/web/SnowySyncService.java (+232/-0)
src/org/tomdroid/sync/web/WebConnection.java (+139/-0)
src/org/tomdroid/ui/Actionbar.java (+69/-0)
src/org/tomdroid/ui/PreferencesActivity.java (+217/-0)
src/org/tomdroid/ui/SyncMessageHandler.java (+158/-0)
src/org/tomdroid/ui/Tomdroid.java (+122/-179)
src/org/tomdroid/ui/ViewNote.java (+36/-4)
src/org/tomdroid/util/NoteContentBuilder.java (+2/-1)
src/org/tomdroid/util/NoteListCursorAdapter.java (+113/-0)
src/org/tomdroid/util/Preferences.java (+112/-0)
src/org/tomdroid/util/XmlUtils.java (+58/-0)
src/org/tomdroid/xml/NoteContentHandler.java (+1/-0)
tests/org/tomdroid/NoteManagerTest.java (+74/-0)
tests/org/tomdroid/NoteTest.java (+35/-0)
To merge this branch: bzr merge lp://staging/~tomdroid-dev/tomdroid/sync-ui
Reviewer Review Type Date Requested Status
Olivier Bilodeau Approve
Review via email: mp+33434@code.staging.launchpad.net

Commit message

Note synchronization support (SD Card and Tomboy Web)

Description of the change

The changes made in the sync-ui branch are of great value to the end users and do not seem to have any obvious bugs. Time to do a 0.4 release!

Not only does the new code introduce a visual progress feedback while syncing, but is also using an Actionbar as lately proposed by the Android developers from Google.

To post a comment you must log in.
Revision history for this message
Olivier Bilodeau (plaxx) wrote :

First, I only glanced quickly at your work but I'm very excited to see your work!

Second, sorry if anything here was already mentionned elsewhere but I haven't replied..

Third, all of this is a matter of discussion. Given the right arguments I will change my mind.

1) order of things
I'm a bit confused with the merge hiearchy here..

Is this based on web-sync meaning we need to merge web-sync first or does this integrates web-sync so merging this will result in a web-sync + sync-ui integration?

2) commons-codec
What is commons-codec used for? Was there no built-in alternative already available? I'm trying to avoid adding a 64k jar into the tomdroid package here.

3) signpost
Not sure what you did with signpost.. haven't looked but can't you use a released jar (with a version we can track bugs against, etc.) instead of having a git checkout of their stuff? Did you need to modify their upstream code? If you can't proceed with released code, can you get rid of the git metadata (signpost/.git/) and identify in a README.tomdroid what revision/repo is this code from and what custom changes you needed done. Also, I would like the eclipse metadata removed.

4) file headers
Some new files are missing the usual project header. Be careful to assign copyright to the right person who created the work. If you are willing to give me the copyright feel free to do so. In any case you will be attributed for your work elsewhere.

Start looking at these please. Once dealt with I'll checkout the branch and play with it.

Thanks!

Revision history for this message
Benoit Garret (benoit.garret) wrote :

It's also my fault the merge hasn't been done before, I haven't followed through with Rodja, mainly due to a lack of time on my part.

1) No new commits have been done on the web-sync branch, so the sync-ui supersedes it. This merge request is the right one.

2) commons-codec is needed by signpost. I pulled everything because it was easier, but I believe only a small set of its functionality is needed. We could rip out the parts that interest us and cut down a lot on the size.

3) When I began working with signpost, a few bugs prevented it from working with snowy and ubuntu one, that's why I kept a patched copy. It's just a matter of looking if there's been a release with the fixes I submitted upstream.

4) As far as I know, only Rodja and I worked on this. I suggest we both go through the files and put the notices on the files we know we created.

234. By Rodja

Modified copyright headers in files I've worked

235. By Benoit Garret

Add copyright and license headers in files I've worked on

236. By Benoit Garret

Removed commons-codec and signpost git, using the jar as bugs with Android have been fixed

Revision history for this message
Benoit Garret (benoit.garret) wrote :

Every point you've (Olivier) mentioned should be solved now. I originally included commons-codec because the version shipped with Android was buggy and messed with the Authorization headers, but signpost got a custom implementation since then.

You're now welcome to play with the branch ;-), I'm really looking forward to the day this is merged.

Revision history for this message
Olivier Bilodeau (plaxx) wrote :

Thanks for addressing my concerns quickly. I'll be trying it on my phone in the next couple of day.

Revision history for this message
Olivier Bilodeau (plaxx) wrote :

Hi guys,

I played with the branch for a couple of minutes tonight. Here are my comments:

5) I upgraded and my notes were all still there. However any attempts to open a note resulted in a parsing error. I'm guessing that something changed about the database. If not db then appending note-content tags or not? I would recommend dropping the database on upgrade. It's easy, notes should be on sdcard because sync never existed and nothing should be only in the db since you can't create content yet (oh and we are not 1.0).

6) I'm also unsure about the sync button.. It was not obvious to me at first that I needed to hit that to sync. It's a bit small and the animation for SD card notes was only a dot blinking in the middle. Is that normal? Nothing turning? But this is minor and could be reworked later.

7) Lastly, I'm unsure about the black text on white background for the note list (I haven't tested notes yet since they crash). It is quite the opposite as most android apps (especially the natives one), well.. at least, on my phone (normal android UI). I might revert these changes unless convinced otherwise (or provide an option?).

By the way, I don't want to sound like I'm dumping all this burden on you guys, I'll look into fixing stuff myself but I don't have time now.

I'll be testing more soon. The sync goodness! I can't wait!

review: Needs Fixing
Revision history for this message
Rodja (trappe) wrote :

Hi Oliver,

Thank you for looking at this branch.

5) I'm not aware of any db schema changes. I guess the issue you experienced happens if you change from sd-card notes to web sync... but I've no time to investigate this further right now. Benoit, could you help?

6a) I have decided to follow Google's blog article about the Twitter App: http://android-developers.blogspot.com/2010/05/twitter-for-android-closer-look-at.html ? They recommend to use the Action bar for common and often used global actions. See http://code.google.com/intl/de-DE/events/io/2010/sessions/android-ui-design-patterns.html at minute 12:40 for the whole Action bar talking, and note at 14:35: "What are the actions the user should not have to press menu to get at". See also the discussion at https://bugs.launchpad.net/tomdroid/+bug/549643

6b) SD card syncing is very fast so the turning animation is rather a quick jump from 0% to 100%.

7) All newer Google Apps like the Gallery, Android Market, Clock, Google Talk, Google Goggles, ... and the thrid party Apps where Google was involved like Amazon MP3 and the offical Twitter App use a light theme. I think this is a trend Tomdroid should follow.

Revision history for this message
Olivier Bilodeau (plaxx) wrote :

Quick note to say that I asked about the color changes to the tomdroid-dev mailing list.

237. By Rodja

droping local database when switching between syc methods

238. By Rodja

Fixed switching back to sdcard sync method.

239. By Rodja

Merged Guilherme Salgado's changes to excludes notebook templates.

Revision history for this message
Rodja (trappe) wrote :

Hi Oliver,

5) I've now implemented a simple drop of the local database when switching sync methods. Should fix your issue.

I'm not aware of any other issues or ToDo's which may prevent merging. Please tell me if anything else must be done.

240. By Rodja

Merged with Matt Stevenson's freshen-ui branch which introduces last change dates in the note list and inserts the tomdroid logo in the actionbar.

241. By Rodja

Made sure the tags member is initalized.

Revision history for this message
Olivier Bilodeau (plaxx) wrote :

Ok, thanks, I'll re-test this week and let you know.

For color and style, based on the feedback, we will move forward with what
you have done.

On Sun, Sep 26, 2010 at 8:16 AM, Rodja <email address hidden> wrote:

> Hi Oliver,
>
> 5) I've now implemented a simple drop of the local database when switching
> sync methods. Should fix your issue.
>
> I'm not aware of any other issues or ToDo's which may prevent merging.
> Please tell me if anything else must be done.
> --
> https://code.launchpad.net/~tomdroid-dev/tomdroid/sync-ui/+merge/33434<https://code.launchpad.net/%7Etomdroid-dev/tomdroid/sync-ui/+merge/33434>
> You are reviewing the proposed merge of lp:~tomdroid-dev/tomdroid/sync-ui
> into lp:tomdroid.
>

--
Olivier Bilodeau <email address hidden>

Revision history for this message
Olivier Bilodeau (plaxx) wrote :

Performed some tests:
- 1.6 emulator SD Card sync - ok
- 1.6 emulator with QA note set (public.img) - ok
- 2.1 emulator SD Card sync - ok
- 2.1 emulator with QA note set (public.img) - ok
- 1.6 ADP1 SD Card sync - ok
- 1.6 ADP1 with personal note set - ok

Tomorrow or saturday I'll be entering release mode. Now, I'm heading for bug triage and building the NEWS file, actually I will need your help for that as I'm afraid some noteworthy changes and contributors will be lost in all the various branches..

Contributors I have for now: Benoit Garret, Rodja Trappe, Matt Stevenson, Guilherme Salgado

NEWS: I'll start something in lp:tomdroid and you guys send me a patch if I'm missing something new or proper attribution.

Anyone tested the recently launched tomboy-online service? I know I will but only after releasing 0.4.0.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches