-
Notifications
You must be signed in to change notification settings - Fork 6
Travis CI: Run on multiple Fedora versions #68
Conversation
😕 |
f5f5486 to
355aa03
Compare
|
So the testing mechanism now works, yet the tests fail on F29+. I'm investigating. Seem like |
3a10ace to
5888d4d
Compare
Now when we have 3.6 and 3.7, this is important
Requestd by us actually, see rpm-software-management/dnf#842
| return self.query.filter(**kwargs).run() | ||
| except AttributeError: | ||
| except AttributeError as ae: | ||
| log.debug(repr(ae)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I seriously don't know why we let AttributeError go, but I don't want to break anything. However this can cover serious problems (it did).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is for rare cases when self.query is None, because get_dnf_query fails to add repo. But it should not let the error go, it should probably somehow nicely say it can't complete the check. But it is out of scope here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. I've tried to make this bit better in 16ca638
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
|
It passes! |
|
|
||
| install: | ||
| - sed -i "s/fedora-28-x86_64/fedora-${FEDORA}-x86_64/" mock.cfg | ||
| - sed -i "s|FROM fedora|FROM registry.fedoraproject.org/fedora:${FEDORA}|" Dockerfile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be changed directly in the files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is. I probably don't understand the question.
The registry.fedoraproject.org/ part is there for f29 to work before the release. I can add it directly to the file but it would make no difference.
irushchyshyn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a question but it looks good overall. Nice work, feel free to merge
|
Thanks for the review! |
Now when we have 3.6 and 3.7, this is important