pagure

#1036 Correct the links of the "Commits tab" when viewed from a commit in another branch than master
Merged by pingou. Opened by vivekanand1101.
vivekanand1101/pagure issue_1031  into  master

Download 1036.patch
no initial comment

ok :thumbsdown: on the changes on this file actually, a commit does not belong to a branch, or rather could belong to several (if branches are merged) making it messy imho.

I had a different understanding of your issue from IRC, I thought you were referring to the drop-down of branches on the header.

In addition to my comment, the PR conflicts :)

On the changes made in that file. I came across that concept here. The problem that was occurring was that the commits page didn't receive any data about which branch it was in, so it didn't have enough data to go to that branch from there.

About the issue, if it's not clear yet: http://img.susepaste.org/82866851

look at the left bottom - the link the browser shows. At that time, the mouse on the commits tab.

Hm, I see what you mean, but I still do not like the fix, a commit is identified by its hash there is no need to provide a branch.

I would be ok with providing the branch as an argument so /c/asdasda?branchname=foo and reuse that info

rebased

rebased

rebased

Would be cool to add a small test for this

rebased

Rather than checking the code/diff, maybe we should check that the link to commits is rightly set?

rebased

2 new commits added

  • Add unit tests for view commit when branch name is provided
  • Correcting links to commits page from any 'commit page' if branch is not master

I was wondering about this abort, since the branch is not actually something that is necessary, maybe we can just set it to None and not return a 404, what do you think?

it will be a little more "friendly" to the user (probably), if it's not aborted.

2 new commits added

  • Add unit test for viewing commit when branch name is wrong
  • Do not abort if branch name is wrong while viewing a commit

wait, do not merge.

Hm, isn't that the default?

4 new commits added

  • Add unit tests for view commit in case the branch name is wrong
  • Do not abort if branch name is wrong while viewing a commit
  • Add unit tests for view commit when branch name is provided
  • Correcting links to commits page from any 'commit page' if branch is not master

2 new commits added

  • modify test for unit test when branch name is wrong: go to commits page in general instead of default branch commits page
  • Do not specifically go to default branch commits page, go to simple commits in general instead, if branch name is wrong

Looks good to me

Pull-Request has been merged by pingou
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.