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

media-gfx/imv: added missing deps for the test phase #1576

Closed
wants to merge 1 commit into from

Conversation

Hummer12007
Copy link
Contributor

CC @gentoo/proxy-maint

@monsieurp
Copy link
Member

I'm not quite sure testing a live package makes sense.

@monsieurp
Copy link
Member

Please provide a rationale as to why a live package needs testing. IMO live packages are too prone to fail to compile in the first place so testing them should be the least of our worries.

@monsieurp monsieurp closed this Jun 3, 2016
@mgorny mgorny reopened this Jun 3, 2016
@mgorny
Copy link
Member

mgorny commented Jun 3, 2016

@monsieurp, please don't close valid pull requests just because you don't care for live packages. They are packages as good as any, and there is no reason not to fix bugs like missing dependencies. Especially that the maintainer may use the live ebuild to write a release ebuild afterwards, and the test dependencies will be useful information to him.

@mgorny
Copy link
Member

mgorny commented Jun 3, 2016

@Hummer12007, is this a new dep or does the current release version ebuild miss them as well? If the latter, please update it too.

@monsieurp monsieurp added the assigned PR successfully assigned to the package maintainer(s). label Jun 3, 2016
@monsieurp
Copy link
Member

The PR is valid. The rationale isn't provided. ;) Anyway, it's yours now.

@Hummer12007
Copy link
Contributor Author

@mgorny, changes necessary for the tests to link correctly had only been recently been shipped into the upstream, but are yet to be released. So tests are restricted for the current version, but still work on the live ebuild. I was waiting for the release, but @gktrk asked me to make the change. I can put the pr on hold for the time being (until the new version gets released).

@gktrk gktrk self-assigned this Jun 3, 2016
@gktrk
Copy link
Member

gktrk commented Jun 3, 2016

@Hummer12007 given the upstream's aperiodic release schedule and that there haven't been many commits to warrant a new release, I will merge this. I also suggest to fix the test deps for 2.1.2 too even though it is test restricted: We don't have to remember adding them in the next bump, we'll simply remove the test restriction. I can squash the 2.1.2 fix on top of this locally before pushing it to the tree unless you insist on doing the work and updating this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s).
Projects
None yet
4 participants