Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crear/merge kb cleanup #55

Closed
wants to merge 25 commits into from
Closed

Crear/merge kb cleanup #55

wants to merge 25 commits into from

Conversation

pardo-bsso
Copy link
Member

Hi all, please review and pull for:

This depends on https://github.com/inaes-tic/mbc-playout/pull/51

josx and others added 25 commits April 1, 2013 16:22
also, restore the "connectable" class to the table, without it
drag behaves funny.

Signed-off-by: Adrian Pardini <[email protected]>
Previously you close the MediaList panel while editing playlists
See #36 y #37. It needs a better look.
Signed-off-by: José Luis Di Biase <[email protected]>
Signed-off-by: José Luis Di Biase <[email protected]>
Before this it wouldn't show on the view.

Signed-off-by: Adrian Pardini <[email protected]>
Adds a dummy dependency to the collection so it updates whenever something changes in it.

Closes #41
I've kept disable_drag just for Browse Medias
When we access other Observables within a ComputedObservable
KO keeps track of that and updates the ComputedObservable accordingly.
Also, remove the total time bar for Browse Media / Left panel.
See discussion at e080b8e

Now we have four types of MediaListView:
  - playlist-sortable (like the current playlist on Edit Playlists)
    has full drag and drop, can edit name and remove elements
  - playlist-draggable (like the left pane on Edit Playlists)
    cannot edit name nor remove elemens, dragging is enabled
  - playlist-searchable (like the one on Browse Medias)

As a side effect the duration bar was restored.
Signed-off-by: José Luis Di Biase <[email protected]>
Signed-off-by: José Luis Di Biase <[email protected]>
Signed-off-by: José Luis Di Biase <[email protected]>
@ghost ghost assigned dgaus Apr 1, 2013
@pardo-bsso pardo-bsso mentioned this pull request Apr 1, 2013
@xaiki
Copy link
Member

xaiki commented Apr 1, 2013

that is a HUGE pull request... please merge often merge early... this is almost impossible to review properly.
@dgaus can you please look through this to see if you see anything fishy ?

for the next time, separate your pull requests between cleanup, refactor, feature work.
separate each feature into a single pull request.
merge often merge early.

thanks.

@xaiki
Copy link
Member

xaiki commented Apr 1, 2013

furthermore,
I see commits that are from the same feature separated by a huge sum of commits (i.e. the 'type' change) this makes it really hard to review.
please use git rebase -i to get your tree in a revieable state before sending pull requests !

@josx
Copy link
Member

josx commented Apr 2, 2013

You are right, next time will be working pull request for each feature

@josx josx mentioned this pull request Apr 2, 2013
@xaiki xaiki closed this Apr 3, 2013
@dgaus dgaus reopened this Apr 3, 2013
@dgaus dgaus closed this Apr 3, 2013
@Lacrymology
Copy link
Contributor

was this dropped for some reason? has it been re-submitted?

@dgaus
Copy link
Contributor

dgaus commented Apr 5, 2013

I believe this was replaced by https://github.com/inaes-tic/mbc-playout/pull/68
please correct me if I'm mistaken

@pardo-bsso
Copy link
Member Author

On 5 April 2013 16:47, Diego Gaustein [email protected] wrote:

I believe this was replaced by #68https://github.com/inaes-tic/mbc-playout/issues/68
please correct me if I'm mistaken

Yes, it was replaced with all the crear/merge/* pull requests.

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

Successfully merging this pull request may close these issues.

5 participants