Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr
1 new commit added
You'll likely want to review this PR commit by commit
you don't really need to create list first, you can use generator..
set(x for x in y) works perfectly and in theory should eat less memory.
set(x for x in y)
Good call, I used the same structure as above but wrongly here, thanks
rebased
You should instead just mock the PDC call out, because right now you are making the test suite depend on an external service, which might not stay around.
Or, as you noticed, suddenly return other data.
I agree but on the other side it allows ensuring the data structure doesn't suddenly change (and it's a single test that will break twice a year or so)
Well, if it changes that'd be a bug in PDC, not Pagure.
Agreed but I'd rather detect this here in the test suite than in prod (though prod will likely break before we run the tests here ^^)
Note that the other tests are mocking the call to PDC so if we were mocking it here as well as would basically be testing the mock works :)
Note that as far as I can find, there has not yet been a conclusion in the "should provenpackagers be able to commit to firefox/xulrunner/thunderbird repos" discussion (https://pagure.io/fesco/issue/1562). We should not be making this decision unilaterally, but await FESCo's decision on this.
If you can find a record of this decision having been made, that would be great to add here.
Adjusted :)
This code looks fine, although I still think you should mock out PDC, but that can be a different PR.
Thanks for the review! :)
Pull-Request has been merged by pingou
Thank you for the quick response. I'll push out the changes to those packages soonish (hopefully later today) and thus give it a test.
Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr