Make rpm dist a renderable property #2313
Conversation
anish
commented
Jul 10, 2016
•
anish
commented
- Also changes default dist from el5 to el6
- Optional rpm build properties are now built up as a dict before being rendered
- Changed tests to cover default value of 'dist'
Current coverage is 85.84%@@ master #2313 diff @@
==========================================
Files 293 293
Lines 30530 30537 +7
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 26206 26213 +7
Misses 4324 4324
Partials 0 0
|
| @@ -88,6 +94,9 @@ def __init__(self, | |||
| 'stdio', logobserver.LineConsumerLogObserver(self.logConsumer)) | |||
|
|
|||
| def start(self): | |||
|
|
|||
| self.rpmbuild = ('%s --define "dist %s"' % (self.rpmbuild, self.dist)) | |||
There was a problem hiding this comment.
It would be nicer to create a new local variable for this, rather than modifying in place. Step instances aren't re-used, but absent that knowledge this looks like every run of the step would add an additional --define dist argument.
There was a problem hiding this comment.
@djmitche That was easy enough to fix, but this pattern repeats itself in the code already. Would it make sense to covert it to a dict instead and then generate command based on that ?
anish
force-pushed
the
rpm_renderable
branch
3 times, most recently
from
ad7563d
to
b46b56f
Compare
|
Good point, yes, I think that would be great if you don't mind. |
|
Already done :)
|
|
Hah, I didn't even look at the changes -- yes, looks nice! |
