sclorg / rpm-list-builder

RPM List Builder helps you to build a list of defined RPM packages including Software Collection from the recipe file
GNU General Public License v2.0
4 stars 8 forks source link

Koji hotfixes #74

Closed khardix closed 7 years ago

khardix commented 7 years ago

Fixes for two issues found when trying to rebuild rh-ror50 with KojiBuilder:

Tests triggering those two issues are included.

khardix commented 7 years ago

@junaruga @hroncok Another round of fixes, this time related to SRPM naming and the necessity to add packages to a destination tag of a build target before the build can be submitted.

khardix commented 7 years ago

Test are failing because scl-utils-build is not present in the test environment, and thus the SCL-related macros are not defined. sclorg#84 should fix this.

khardix commented 7 years ago

Rebased onto current master.

junaruga commented 7 years ago

@khardix I have a question. There are 4 points using is None. Those are below case? Do you want to distinguish empty string/zero and None?

$ grep -r 'is None' rpmlb/builder/koji.py 
        if koji_epel is None:
        if koji_target_format is None:
        if self.profile is None:
        if position is None:

http://docs.python-guide.org/en/latest/writing/style/#check-if-variable-equals-a-constant

or, since None is considered false, explicitly check for it

if attr is None:
   print 'attr is None!'

Others look good to me.

khardix commented 7 years ago

@junaruga In most of them, yes.

The first two (actually three, this PR add one more) are handling default arguments to __init__ and need to know if the default should be used.

The self.profile one could be changed to if not self.profile, since empty profile name could be seen as no profile, but I would rather leave it as it stands (personal preference).

The position indicates position in bootstrap sequence, and position 0 != no position.


Note: I'm working on another change found during rh-ror50 rebuild, so no need to review yet.

khardix commented 7 years ago

I'm pleased to announce that after the last commit, the rh-ror50 collection was completely built in CBS.

That means this PR is no longer WIP, and ready for full review :tada:

@junaruga @hroncok Who will volunteer?

junaruga commented 7 years ago

I am fine for current code. But I want to ask to hear @hroncok 's opinion.

khardix commented 7 years ago

History rewritten to more logical chunks, and rebased on top of the current master. After Travis passes, I will merge this.