pagure

Clone

#2361 Fix diffing local branch against the target one
Merged 6 years ago by pingou. Opened 6 years ago by pingou.
diff_master  into  master

file modified
+10 -3 
@@ -262,10 +262,16 @@ 
 

  

      reponame = pagure.get_repo_path(repo)
 

      repo_obj = pygit2.Repository(reponame)
 

+     if repo.is_fork:
 

+         parentreponame = pagure.get_repo_path(repo.parent)
 

+         parent_repo_obj = pygit2.Repository(parentreponame)
 

+     else:
 

+         parent_repo_obj = repo_obj
 

  

      branches = {}
 

      if not repo_obj.is_empty and repo_obj.listall_branches() > 1:
 

-         if not repo_obj.head_is_unborn:
 

+         if not parent_repo_obj.is_empty \
 

+                 and not parent_repo_obj.head_is_unborn:
 

              compare_branch = repo_obj.head.shorthand
 

          else:
 

              compare_branch = None
 
@@ -273,14 +279,15 @@ 
 

          for branchname in repo_obj.listall_branches():
 

  

              # Do not compare a branch to itself
 

-             if compare_branch \
 

+             if not repo.is_fork \
 

+                     and compare_branch \
 

                      and compare_branch == branchname:
 

                  continue
 

  

              diff_commits = None
 

              try:
 

                  _, diff_commits, _ = pagure.lib.git.get_diff_info(
 

-                     repo_obj, repo_obj, branchname, compare_branch)
 

+                     repo_obj, parent_repo_obj, branchname, compare_branch)
 

              except pagure.exceptions.PagureException:
 

                  pass

file modified
+2 -2 
@@ -86,7 +86,7 @@ 
 

              <div class="m-b-2">
 

                {% if head %}
 

                <div id="branch-{{ head }}" class="repoinfo-branchlistitem row">
 

-                 <div class="branch_name col-md-10">
 

+                 <div class="branch_name col-md-8">
 

                    {% if branchname == head %}
 

                    <span class="oi text-muted" data-glyph="random"
 

                      title="Default branch"></span>
 
@@ -104,7 +104,7 @@ 
 

                    </a>
 

                    {% endif %}
 

                  </div>
 

-                 <div class="branch_del col-md-2">
 

+                 <div class="branch_del col-md-4">
 

                    <span class="oi text-muted" data-glyph="star"
 

                      title="Default branch"></span>
 

                  </div>

no initial comment

Needs #2362 and #2352 merged before

The Jenkins tests seem to be unable to get connections to the database.

I recommend documenting the type of backend here.

It would be good to add a docblock to this class.

It would be good to add a docblock to this method.

Would this ever be used on large files? It could cause memory issues since it loads the whole file into memory at once.

I recommend documenting each of the parameters and their types.

This method is pretty long and a bit hard to follow since it uses many levels of indentation at points. I recommend breaking it up into smaller helper functions. That should also make it easier to test.

I don't see cls being used below - you could convert this to a @staticmethod.

It might be important to check on the exit code of this process. If it fails that could lead to unexpected behaviors in Pagure, but without exit code checking and logging, it won't be obvious that this cmd was the problem.

You can also make this one a @staticmethod.

This could also be static, and it would be nice to add a docblock.

Maybe drop this print statement.

LGTM

okido

Note that it looks like you reviewed mainly https://pagure.io/pagure/pull-request/2352 :)

Done

Done

It's the test backend, so all it's doing is this print statement :)

Done

Good idea

Done

Done

Done

It may but I expect these files will be fairly small. The method is used to read the files to include at the top/bottom of the gitolite config file.

And note that these files are configured in the configuration file of pagure, not by the users

Turns out if I do I break the inheritance since this method is called in generate_acls and overriden in Gitolite3Auth

Scratch that I think that was me

rebased
6 years ago

looks good :bikini:

Thanks everyone! :)

Pull-Request has been merged by pingou
6 years ago
Changes Summary 2
+10 -3
file changed
pagure/internal/__init__.py
+2 -2
file changed
pagure/templates/repo_info.html
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.