pagure

Clone

#4549 lib/git.py: Recognize git submodules when obtaining patch stats
Opened 5 years ago by msrb. Modified 5 months ago
msrb/pagure fix-unboundlocalerror  into  master

Merge
file modified
+3 -2 
@@ -2744,8 +2744,9 @@ 
 

      elif hasattr(patch, "delta"):
 

          status = None
 

          # Newer pygit2
 

-         # we recognize non-executable file, executable file and symlink
 

-         expected_modes = [33188, 33261, 40960]
 

+         # we recognize non-executable file, executable file,
 

+         # symlink and a special "git submodule" directory
 

+         expected_modes = [33188, 33261, 40960, 57344]
 

          if (
 

              patch.delta.new_file.mode == 0
 

              and patch.delta.old_file.mode in expected_modes

file modified
+2 -2 
@@ -17,8 +17,8 @@ 
 

  {% endblock %}
 

  {% block repo %}
 

  {% set splitted_message = commit.message.split('\n') %}
 

- {# we recognize non-executable file, executable file and symlink #}
 

- {% set expected_modes = [33188, 33261, 40960] %}
 

+ {# we recognize non-executable file, executable file, symlink and a special "git submodule" directory #}
 

+ {% set expected_modes = [33188, 33261, 40960, 57344] %}
 

  <div class="row">
 

    <div class="col">
 

        {% block overviewtabs %}{{ super() }}{% endblock %}

Fixes:
UnboundLocalError: local variable 'status' referenced before assignment

Our OSCI Pagure instance was returning 500 when there was a pull request which was adding a new git submodule into the repository. This patch fixed the problem.

Note I'd love to add some tests, but I am gonna need some guidance on where exactly those tests should go.

Thanks! 

07:24:57  Failed tests:
07:24:57  FAILED test: py3-test_style

@msrb Could you please run black on your code to fix the style?

@ngompa absolutely! Thanks.

rebased onto f89713e5a0ffce715381c01cc3cdb8abe8c2a5a5
5 years ago

So the problem was:

$ python -m flake8 --ignore=E712,W503,E203 pagure/
pagure/lib/git.py:2747:80: E501 line too long (108 > 79 characters)

It should be fixed now. I force-pushed the fix, hope it's ok.

That's fine, we'll see if the tests agree. :wink:

They don't :) Let me try to figure out what is going on there...

Now the problem is not on your side, we have a broken pipeline due to some f30/pip updates

@msrb can you rebase the pr? the test problem should be fixed now

rebased onto 547c67a
5 years ago

@jlanda rebased :wink: let's see now...

We are green, folks! \o/

:thumbsup:

For the tests, I'd say feel free to add them to a new file :)

@msrb Do you need some help to get started on the tests?

Sorry for the delays guys - I will try to finalize this PR by the end of this week.

@msrb Any effort going into this now?

I wanna land this on next release. Can I finish the tests @msrb ?

Metadata Update from @jlanda:
- Request assigned
4 years ago

I'll took this silence as a yes :)

@jlanda Any chance you're planning on taking a crack at this soon?

pretty please pagure-ci rebuild
3 years ago

pretty please pagure-ci rebuild
3 years ago

@ngompa is that something we want to have in the next release? If so I can create a new PR from the patch and write the missing tests.

@wombelix Yes, this is something we want to have. Being able to properly recognize submodules would be very useful.
Changes Summary 2
+3 -2
file changed
pagure/lib/git.py
+2 -2
file changed
pagure/templates/commit.html
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.