971704 - Clean up Wallpaper File Structure in Gaia

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect, P1)

Description

[Patryk Adamczyk [:patryk] UX]

Currently the file structure is messy and confusing, the gaia wallpaper folder looks like:

https://github.com/mozilla-b2g/gaia/tree/master/apps/wallpaper/resources

There are 2 folders: 
+ 320x480
+ 480x800

I believe these 2 folder can be removed, and everything from "320x480" folder should be moved into the root folder: "resources".

Also the "320x480" folder contains old and unused wallpapers those should also be removed:
- leaves.png
- [e-mail]
- [e-mail]
- morningdew.png
- [e-mail]
- [e-mail]
- [e-mail]
- water.png
- [e-mail]
- [e-mail]

Everything in the "480x800" folder can be deleted.

Comment 1

[Joshua Smith [:joshua-s]]

What code would need to be updated to deal with this change?

Comment 2

[David Flanagan [:djf]]

Joshua: are you volunteering? I'd love your help :-)

The "list.json" file in the same directory as the wallpapers would have to be updated.

And the code in apps/wallpaper/js that reads list.json would have to be updated to reflect the new path.

Punam: If Joshua wants this bug, will you help him if he has questions and review his patch when it is ready?  And if he doesn't want to take it, will you add it to your list, please?  Note that this is low-prioirty.

Patryk: It looks like we don't have a full set of @2x images, but I think that the nexus-4 uses them. If you attach the missing images to this bug, we can get them in as part of the cleanup.

Comment 3

[Patryk Adamczyk [:patryk] UX]

Helen Huang, will attach 2x images in the next few days to this bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=949024

Comment 4

[Joshua Smith [:joshua-s]]

(In reply to David Flanagan [:djf] from comment #2)
> Joshua: are you volunteering? I'd love your help :-)

Yes :)

Comment 5

[Punam Dahiya [:pdahiya]]

Thanks Joshua for taking it, please feel free to set review flag or need info for me if you have any questions.

Comment 6

[Joshua Smith [:joshua-s]]

Attachment: PR on GitHub

Comment 7

[Punam Dahiya [:pdahiya]]

Comment on attachment 8376477 [details] [review]
PR on GitHub

Hi Joshua

I see attached PR has below files checked in.
apps/browser/js/init.json
apps/communications/contacts/config.json
apps/communications/contacts/oauth2/js/parameters.js

My understanding is these files are not needed for this fix. Can you pl. look into it and update PR accordingly. Thanks

Comment 8

[Joshua Smith [:joshua-s]]

Oh no :(  I really don't understand this.  Maybe my fork wasn't up to date with Mozilla's or something (although I thought I re-pulled).

Rebasing...

Comment 9

[Joshua Smith [:joshua-s]]

Comment on attachment 8376477 [details] [review]
PR on GitHub

Try it now.

Comment 10

[Punam Dahiya [:pdahiya]]

Thanks Joshua, patch looks good. As noted by Patryk in Comment #3, it will be great if we can include the new sizes in 949024 as part of this clean up. 

Updating the bug with dependency on bug 949024.

Comment 11

[Punam Dahiya [:pdahiya]]

Hi Joshua
With fix of bug 949024, we have new wallpapers. If you have bandwidth, can you please include these updates in your patch and submit for review. Thanks!

Comment 12

[Joshua Smith [:joshua-s]]

I can do that!

Comment 13

[Joshua Smith [:joshua-s]]

I updated the patch to include those changes.  Review please!

Comment 14

[Patryk Adamczyk [:patryk] UX]

Punam, can you please review? Thanks

Comment 15

[Punam Dahiya [:pdahiya]]

Hi Patryk
Sorry for the delay. I will review the bug this week right after the blocker bug i am working on right now. Thanks

Comment 16

[Punam Dahiya [:pdahiya]]

Hi Joshua

The attached patch looks good. I noticed few build scripts are referencing the old wall paper path
./apps/wallpaper/build/build.js:WallPaperAppBuilder.prototype.WALLPAPER_PATH = 'resources/320x480';
./build/test/integration/distribution.test.js:    helper.checkFileContentByPathInZip(zipPath, 'resources/320x480/list.json',
./build/test/integration/distribution.test.js:      'resources/320x480/customize.png',

Can we update new wallpaper paths for above build files. With that fixed you have my r+ and patch is good to land on master.

Thanks
Punam

Comment 17

[Punam Dahiya [:pdahiya]]

Hi Kevin
I am not familiar with builds, so i am including you in the loop for the change in build files (#comment 16) as part of clean up of wall paper file structure in gaia. 
Thanks!

Comment 18

[Joshua Smith [:joshua-s]]

Oh, I didn't catch those.  Thanks!

I did update the Travis tests for the wallpaper app.  Is there anyone else who needs to be cc'd for that change?

Comment 19

[Punam Dahiya [:pdahiya]]

(In reply to Joshua Smith [:joshua-s] from comment #18)
> Oh, I didn't catch those.  Thanks!
> 
> I did update the Travis tests for the wallpaper app.  Is there anyone else
> who needs to be cc'd for that change?

Thanks Joshua, i reviewed the unit tests and those changes look good.

Comment 20

[Joshua Smith [:joshua-s]]

The files you listed were updated!  I searched the rest of the repository to find other instances of the old url, and there were none.  Should be ready to merge :)

Comment 21

[Punam Dahiya [:pdahiya]]

Comment on attachment 8376477 [details] [review]
PR on GitHub

Thanks Joshua, r+ from me. Please let me know if you need me to merge patch on master.

Comment 22

[Kevin Grandon :kgrandon]

I am going to forward the ni? over to Yuren to see if anything needs to be updated here.

Yuren - also if there is any process for notifying partners about this change or updating some documentation it would be good to ensure we don't miss anything. Thanks!

Comment 23

[Joshua Smith [:joshua-s]]

@punam: yes, I will need someone to merge it for me.  Is there a way that contributors can get push access to Mozilla repos, or is it only for employees?

Comment 24

[Kevin Grandon :kgrandon]

Generally it's easier to just specify "checkin-needed" as a keyword, and it will automatically be checked in once the patch is reviewed. Contributors can get commit access though after a few patches: https://www.mozilla.org/hacking/commit-access-policy/

Thanks for your contribution!

Comment 25

[Joshua Smith [:joshua-s]]

(In reply to Kevin Grandon :kgrandon from comment #24)
> Generally it's easier to just specify "checkin-needed" as a keyword, and it
> will automatically be checked in once the patch is reviewed. 

Done, thanks!

> Contributors can get commit access though after a few patches:
> https://www.mozilla.org/hacking/commit-access-policy/

I have a few questions about this, but don't want to get too off-topic with the bug.  I will ping you in IRC sometime to discuss them.

Comment 26

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

it's not easy to contact each OEM so send an email to dev-gaia will help, and I also needinfo? Albert who is in charge of customization work for telefonica to know this change.

You also need to modify GAIA_DIR/customization/wallpaper/list.json because default wallpapers will be changed.

Albert, this change could affect telefonica's customization repository, could you take a look?

Comment 27

[Albert [:albert]]

(In reply to Yuren [:yurenju][:小朱] from comment #26)
> it's not easy to contact each OEM so send an email to dev-gaia will help,
> and I also needinfo? Albert who is in charge of customization work for
> telefonica to know this change.
> 
> You also need to modify GAIA_DIR/customization/wallpaper/list.json because
> default wallpapers will be changed.
> 
> Albert, this change could affect telefonica's customization repository,
> could you take a look?

Sure, thank you for pointing it out.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/53d9e0608345c63be4faa225ecbbf6ee4dd9e5de