Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix svn update #4933

Merged
merged 1 commit into from Nov 29, 2018
Merged

Fix svn update #4933

merged 1 commit into from Nov 29, 2018

Conversation

@stsewd
Copy link
Member

@stsewd stsewd commented Nov 28, 2018

This is command is making the build half-fail and returning 0.

@codecov
Copy link

@codecov codecov bot commented Nov 28, 2018

Codecov Report

Merging #4933 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master    #4933   +/-   ##
=======================================
  Coverage   76.75%   76.75%           
=======================================
  Files         158      158           
  Lines       10048    10048           
  Branches     1265     1265           
=======================================
  Hits         7712     7712           
  Misses       1995     1995           
  Partials      341      341
Impacted Files Coverage Δ
readthedocs/vcs_support/backends/svn.py 29.5% <0%> (ø) ⬆️

@stsewd stsewd requested a review from Nov 28, 2018
@stsewd
Copy link
Member Author

@stsewd stsewd commented Nov 28, 2018

This is working locally

@stsewd
Copy link
Member Author

@stsewd stsewd commented Nov 28, 2018

Codecov failure is expected, we don't have tests for svn.

@@ -39,7 +39,7 @@ def update(self):
super(Backend, self).update()
# For some reason `svn status` gives me retcode 0 in non-svn
# directories that's why I use `svn info` here.
retcode = self.run('svn', 'info', record_as_success=True)[0]
retcode, _, _ = self.run('svn', 'info', record=False)
Copy link
Member

@humitos humitos Nov 29, 2018

Why we don't want to record_as_success=True here?

Copy link
Member Author

@stsewd stsewd Nov 29, 2018

The command is only used to check if the repo exists, so not relevant to the user. as_success makes the command to always return 0 as exit code, so no something we want.

Copy link
Member

@ericholscher ericholscher left a comment

Looks good. 👍

@ericholscher ericholscher merged commit a5a761b into readthedocs:master Nov 29, 2018
2 of 3 checks passed
@stsewd stsewd deleted the fix-svn branch Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants