#254 Fix for Issue #106
Closed by pingou. Opened by subho.
subho/pagure Fix  into  master

Download 254.patch
no initial comment

@pingou , Here's screenshot for how the frontend looks.

Branch Change Screenshot: http://i.imgur.com/dTxsJZT.png

Currently In new repositories the default branch was set to 'master' whether the project had any master branch or not. Therefore in new repositories I have set the default Branch as 'None' .

New repo Screenshot: http://i.imgur.com/T30Qyc8.png

I haven't used wtforms for the 'default_branch_form' here, let me know if you want it using wtforms.

I think this would be better in the settings page of the project

None as a string?

None as a string?

wtforms should be used at least to provide CSRF protection

print will break in production

Why this change? :)

Also, might be nice if we could add some unit-tests for this :)

Why this 'None' here?

Hi @pingou, as we discussed on IRC , I am running on Fedora 22 and I have pygit2 (0.22.0) which is causing test cases to fail. (Though i wonder how the normal application functioning works fine but testcases fail). So i wont be able to add the unit-tests for this patch until this issue is fixed.

Though i wonder how the normal application functioning works fine but testcases fail

It's easy, it doesn't :)

The application runs on EL6 and Fedora 21 fine, both have pygit 0.21, so we need to fix it 0.22 while keeping backward compatibility with 0.22.

Hi @subho

If you rebase your work on the branch pygit_022 you should be able to run the test suite.

@pingou, oh okay, will do that. But for some reason i don't get email notifications when you comment here. I just happen to check this link once in a while for updates and see your comments.

@subho this is odd, you should be getting those emails :-/

@pingou, added the feature setting default branch and the tests for the same.

Something of interest for this line: https://github.com/libgit2/pygit2/pull/540#issuecomment-127023104

As far as i understand they are going to remove the message parameter for the set_head() function [1] in next version which shouldn't affect this code here since i am not passing any message here. Please let me know if i missed out on anything here.

[1] https://github.com/libgit2/pygit2/pull/540/files#diff-57507982bc2ad39925ca8f387e172b29L295

You should check the configuration of your IDE so that it removes the lines containing just spaces ;-)

1 line too many here :)

This is working really nice!

Just maybe:
- Set the branch to X, the overview shows the commit in X, click on the branch X (so you see the commits in that branch), the page shows that there are commits in X that are not in master and offers to create a PR
- When creating a PR, X is not the default branch proposed (same for remote PR)

These are the two points I can see now, will check further if there are others place to change.

@subho do you want to work on these, or do you want me to? :)

@pingou, i am willing to work on these. :smiley:

Ok there are still some things missing:

  • Set the default branch to X
  • View branch Y
  • It shows the commits that are different between Y and master, not between Y and X (Same thing if you see the commits of branch X, it offers you the option to create a PR)

  • On the Remote PR page, the default branch, isn't X

Create a repo with only a branch X (no master, no Y)

  • On the Doc page, the link to the Tree view does not specify to which branch
  • On the Commits page for branch X (not the overview, the Commits), the Request Pull button is broken (as only ..X instead of X..X and there should actually be no button at all).

@pingou , about the last point above, where the Request pull button is broken, i think a "open remote pull-request" button should be there. What do you think?

The remote PR yes, it should still be there.

@pingou , updated the PR.

Hi @subho, I have been reviewing and testing it, it looks great! :)

I'll merge manually though to keep the history linear but thanks a lot for your work!! :)

Metadata