#2422 Simplify diff calculation
Merged by r3pek. Opened by r3pek.
https://code.r3pek.org/r3pek/Pagure.git master

Download 2422.patch
no initial comment

The first diff really doesn't make much sense.

One thing that I didn't test this commit with was with a commit that has multiple parents, but from the look of it, it should work the same way.

rebased

Wow, so this PR breaks the test suite and it took me really a while to find out why.

The issue is in https://pagure.io/pagure/pull-request/2422#_2,5-13 and the reason for this is that due to the URL routing in flask and our URL schema (that's more the actual issue) there are cases were the identifier can be None. So if we do not check for this identifier, we may display an diff where we should raise a 404, which is what this test caught: https://pagure.io/pagure/blob/master/f/tests/test_pagure_flask_ui_repo.py#_2014-2015

So I will be reverting this part of the patch before merging it.

Ah! My bad @pingou

I really thought that https://pagure.io/pagure/blob/master/f/pagure/ui/repo.py#_590 was testing that and in the case the identifier was None it would be caught in the "else" part. That's why I cleaned it up.

Commit dbd6dd1d fixes this pull-request

Pull-Request has been merged by r3pek@r3pek.org

Metadata