libubuntuone

Merge lp://staging/~jblount/libubuntuone/classy-error-pages into lp://staging/libubuntuone

classy-error-pages
Merge into trunk

Proposed by Joshua Blount on 2010-03-17

Status:

Rejected

Rejected by:

Stuart Langridge on 2010-03-22

Proposed branch:

lp://staging/~jblount/libubuntuone/classy-error-pages

Merge into:

lp://staging/libubuntuone

Diff against target:

621 lines (+521/-7)

7 files modified

data/Makefile.am (+4/-1)
data/connecting.html (+34/-0)
data/in_development.html (+34/-0)
data/load_error.html (+41/-0)
data/reset.css (+49/-0)
data/screen.css (+336/-0)
libubuntuone/u1-music-store.c (+23/-6)

To merge this branch:

bzr merge lp://staging/~jblount/libubuntuone/classy-error-pages

Medium

Fix Released

Link a bug report

Reviewer	Date Requested	Status
Stuart Langridge (community)		Disapprove on 2010-03-22
Rodrigo Moya (community)		Needs Fixing on 2010-03-18
Eric Casteleijn (community)	2010-03-17	Approve on 2010-03-17
Review via email: mp+21513@code.staging.launchpad.net

Description of the change

Adds some css and html to make the error pages look nice / right / good.

Revision history for this message

Rodrigo Moya (rodrigo-moya) wrote on 2010-03-17:

Looks ok to me, although I just merged another branch that adds a new page (initial page when loading the real store), so I think this is going to have conflicts, so can you please merge with trunk and add that other page please?

Revision history for this message

Eric Casteleijn (thisfred) wrote on 2010-03-17:

Looks good, code builds.

review: Approve

lp://staging/~jblount/libubuntuone/classy-error-pages updated on 2010-03-17

55. By Joshua Blount on 2010-03-17: merged with trunk

Revision history for this message

Joshua Blount (jblount) wrote on 2010-03-17:

> Looks ok to me, although I just merged another branch that adds a new page
> (initial page when loading the real store), so I think this is going to have
> conflicts, so can you please merge with trunk and add that other page please?

I merged with trunk, but could you please once over it and make sure I didn't get rid of anything?

Revision history for this message

Rodrigo Moya (rodrigo-moya) wrote on 2010-03-18:

Ok, there are a few problems:

* in_development.html seems to be missing, right? Also, the call to load that file seems to be missing also.
* There are some leaks in the C code:

552 + calculated_file_url = g_strdup_printf("%s?%s",
553 + g_filename_to_uri(calculated_file_path, NULL, NULL),
554 + reload_url);

g_filename_to_uri returns a newly-allocated string, so you need to store it in a variable, use it to g_strdup_printf calculated_file_url, and then g_free it.

* also, we use the GNOME coding style for couchdb-glib (because it's going to be included in GNOME soon), so could you please add a space after the function name and before the '(' character?

Apart from that it looks ok, so please fix that and I'll approve it

review: Needs Fixing

lp://staging/~jblount/libubuntuone/classy-error-pages updated on 2010-03-19