Skip to content

MPV: new package#1381

Merged
7c6f434c merged 2 commits intoNixOS:masterfrom
AndersonTorres:master
Dec 15, 2013
Merged

MPV: new package#1381
7c6f434c merged 2 commits intoNixOS:masterfrom
AndersonTorres:master

Conversation

@AndersonTorres
Copy link
Member

MPV is a movie player, derived from MPlayer/mplayer2, with great improvements.

There are some minor issues, listed in the last four TODO comment lines.

@bjornfor
Copy link
Contributor

Hi, I saw on IRC that you wondered why postInstall wasn't run. It's because you have overridden installPhase. The default installPhase runs preInstall, does the default install action, and then runs postInstall. When you set installPhase yourself you must handle pre/post actions yourself. So just merge postInstall into installPhase.

Can you do a git rebase --interactive and squash all commits into just one commit (and then force push)? That would give a more easy to read history.

@vcunat
Copy link
Member

vcunat commented Dec 14, 2013

Speaking of easier-to-read history: I use a git alias for log that uses --first-parent, so I only see commits included directly into the main branch and merge commits. If I want to inspect what merge did, I do it separately. It seems more pleasant, but it can be personal.

@bjornfor
Copy link
Contributor

@vcunat: Thanks for the tip about --first-parent. Hm, it doesn't seem to work with tig, which is the tool I use for browsing git history.

But I still think pull requests should contain as little noise as possible :-)

It certainly needs a bit of improvement and testing, but it works
nicely - so it's a good time to put it to the great public!
@AndersonTorres
Copy link
Member Author

Thanks! I will do that. In fact I am a bit novice on Git.

It certainly needs a bit of improvement and testing, but it works
nicely - so it's a good time to put it to the great public!
@AndersonTorres
Copy link
Member Author

Now the git pollution was solved!

Also: even if I consider it production-stable, my perfectionist mind can't live with the problems listed at TODO flags.

7c6f434c added a commit that referenced this pull request Dec 15, 2013
@7c6f434c 7c6f434c merged commit 5a333ab into NixOS:master Dec 15, 2013
@bjornfor
Copy link
Contributor

Oh, silly me, "tig --first-parent" works just fine. I was just in too much of a hurry to notice. (Thanks @vcunat)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants