redhat-openstack / easyfix

6 stars 5 forks source link

Remove **BuildRequires/Requires: python-coverage** from spec file #11

Closed chkumar246 closed 6 years ago

chkumar246 commented 7 years ago

python-coverage is used to generate test coverage report for any project. During RPM building process, we run tests only and donot generate coverage report. In RDO Packages, at most places python-coverage is used as BuildRequires or Requires.

What to improve:

  1. Clone tripleo-validations-distgit spec file git clone https://github.com/rdo-packages/tripleo-validations-distgit
  2. Go to tripleo-validations-distgit and open openstack-tripleo-validations.spec in editor.
  3. You can find Requires: python-coverage OR/AND BuildRequires: python-coverage in spec file.
  4. We need to remove those lines from spec file.
  5. Once done! Make changes, add and commit it and send a Gerrit Review.
  6. Please cross checklist before sending Gerrit Review.

Example Spec containing the above fix:

  1. python-os-traits

Below is the list of packages which need same fixes:

Mentors

  1. chandankumar
OGtrilliams commented 7 years ago

test builds failed for keystoneclient & tripleo-validations, but mistrial-dashboard worked:

https://review.rdoproject.org/r/#/c/7742/ https://review.rdoproject.org/r/#/c/7747/

mrunge commented 7 years ago

IMO, coverage should not be removed from horizon spec file. Tests are not run there during package build (different issue), but if you're running the tests, coverage gives a bit more detail.

snecklifter commented 7 years ago

@chkumar246 can you discuss with other cores please as I'm just doing the work. Cheers.

amoralej commented 7 years ago

My opinion on this is:

  1. In packages not running tests with coverage in %check we can remove coverage as BuildRequires
  2. coverage is optional when running unit tests manually so if a user wants to run tests with coverage, he must install python-coverage manually and run it. So i think python-coverage should not be Requires in -tests package as general rule.
javierpena commented 7 years ago

Agreed with amoralej's comment. If coverage is not being actively used in %check, it doesn't make any sense to keep it as a build requirement (because it's not).