1156625 - [Messages] ThreadList&Thread separation: separate JS files

Status: RESOLVED FIXED

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

Description

[Oleg Zasypkin [:azasypkin]]

In this bug we should move ThreadList and Thread specific JS files from :/js" folder to "views/thread-list/js" and "views/thread/js" respectively.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] azasypkin:bug-1156625-thread-list-and-thread-split-js > mozilla-b2g:master

Comment 2

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8595153 [details] [review]
[gaia] azasypkin:bug-1156625-thread-list-and-thread-split-js > mozilla-b2g:master

What do you guys think about such separation? 

Here I extract ThreadList and Thread-Composer from the common js folder, trying to keep it simple as possible.

Comment 3

[Steve Chung [:steveck]]

Comment on attachment 8595153 [details] [review]
[gaia] azasypkin:bug-1156625-thread-list-and-thread-split-js > mozilla-b2g:master

Simple and clear, thanks!
BTW just an idea about the view naming: Do you think thread-list/thread is clear for the views? I think thread-list/message-list is more clear or even thread/message in short. Just an idea for the folder name first and maybe we can polish it while in further separation. It's because the original naming of view seems not very clear for the people not familiar with the code, even UX/QA are confused about the thread/thread list name sometimes.

Comment 4

[Julien Wajsberg [:julienw]]

Comment on attachment 8595153 [details] [review]
[gaia] azasypkin:bug-1156625-thread-list-and-thread-split-js > mozilla-b2g:master

looks about right but the unit tests needs to be moved accordingly to their app counterparts.

Comment 5

[Oleg Zasypkin [:azasypkin]]

(In reply to Steve Chung [:steveck] from comment #3)
> Comment on attachment 8595153 [details] [review]
> [gaia] azasypkin:bug-1156625-thread-list-and-thread-split-js >
> mozilla-b2g:master
> 
> Simple and clear, thanks!
> BTW just an idea about the view naming: Do you think thread-list/thread is
> clear for the views? I think thread-list/message-list is more clear or even
> thread/message in short. Just an idea for the folder name first and maybe we
> can polish it while in further separation. It's because the original naming
> of view seems not very clear for the people not familiar with the code, even
> UX/QA are confused about the thread/thread list name sometimes.

Both "thread-list/message-list" and current "thread-list/thread" look OK for me :) Maybe I just used to it, but I think we still need to keep this "-list" part. 

Let's ask 3rd opinion to be sure we all agree, Julien what do you prefer?

Votes:

thread-list/thread - Oleg+;
thread-list/message-list - Steve+; Oleg+;
thread/message - Steve+;

Comment 6

[Julien Wajsberg [:julienw]]

Another proposal:

inbox/conversation

(don't thank me :) )

Comment 7

[Oleg Zasypkin [:azasypkin]]

Okay, I'll leave it as is for now :) Let's discuss together during review.

Comment 8

[Steve Chung [:steveck]]

Jenny said she would prefer "inbox/conversation" if we're willing to change(And she is happy if we can do so :))

Comment 9

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8595153 [details] [review]
[gaia] azasypkin:bug-1156625-thread-list-and-thread-split-js > mozilla-b2g:master

Hey Julien and Steve, I think PR is ready for the first review round :)

Thanks!

Comment 10

[Steve Chung [:steveck]]

Comment on attachment 8595153 [details] [review]
[gaia] azasypkin:bug-1156625-thread-list-and-thread-split-js > mozilla-b2g:master

Hi Oleg, I left some comments on github but looks great overall, we can have a brand new naming for each view now \o/

Comment 11

[Julien Wajsberg [:julienw]]

This looks good, I just want to experiment a little on the phone but I don't have any comment so far.

Comment 12

[Julien Wajsberg [:julienw]]

Comment on attachment 8595153 [details] [review]
[gaia] azasypkin:bug-1156625-thread-list-and-thread-split-js > mozilla-b2g:master

ok, I found no issue, let's do this :)

I feel a little sad when I think to all my in-progress patch that I'll need to rebase ;)

Comment 13

[Oleg Zasypkin [:azasypkin]]

Thanks for review guys!

> I feel a little sad when I think to all my in-progress patch that I'll need to rebase ;)

Yeah, you're not alone here :)

Comment 14

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/4524078f2dd98e0eb84031b9aa0a81a20b8977cf