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

Only use remote branches for our syncing. #4984

Merged
merged 3 commits into from Dec 10, 2018
Merged

Only use remote branches for our syncing. #4984

merged 3 commits into from Dec 10, 2018

Conversation

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Dec 10, 2018

We don't care about branches local to us.
This causes bugs where a local and remote branch have the same name,
and we sync stable in and endless loop.

Fixes #4980

We don't care about branches local to us.
This causes bugs where a local and remote branch have the same name,
and we sync `stable` in and endless loop.

Fixes #4980
@codecov
Copy link

@codecov codecov bot commented Dec 10, 2018

Codecov Report

Merging #4984 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4984      +/-   ##
==========================================
- Coverage    76.8%   76.76%   -0.05%     
==========================================
  Files         158      158              
  Lines        9937     9937              
  Branches     1241     1241              
==========================================
- Hits         7632     7628       -4     
- Misses       1973     1974       +1     
- Partials      332      335       +3
Impacted Files Coverage Δ
readthedocs/vcs_support/backends/git.py 81.64% <100%> (-0.64%) ⬇️
readthedocs/projects/tasks.py 70.59% <0%> (-0.61%) ⬇️

@codecov
Copy link

@codecov codecov bot commented Dec 10, 2018

Codecov Report

Merging #4984 into master will decrease coverage by <.01%.
The diff coverage is 75%.

@@            Coverage Diff            @@
##           master   #4984      +/-   ##
=========================================
- Coverage    76.8%   76.8%   -0.01%     
=========================================
  Files         158     158              
  Lines        9937    9940       +3     
  Branches     1241    1242       +1     
=========================================
+ Hits         7632    7634       +2     
  Misses       1973    1973              
- Partials      332     333       +1
Impacted Files Coverage Δ
readthedocs/vcs_support/backends/git.py 81.98% <75%> (-0.3%) ⬇️

@ericholscher ericholscher requested a review from Dec 10, 2018
@ericholscher
Copy link
Member Author

@ericholscher ericholscher commented Dec 10, 2018

I've done a somewhat hacky thing here and made it so we use the local branches in testing. I looked at fixing it, but I think we should ship the hacky thing for now to get this working in prod. In the future we can remove it. I will add another ticket for this.

Copy link
Member

@humitos humitos left a comment

This looks fine to me.

Did you already hotfixed this and we are sure that this will work in production? After some local testing I think it will, but I'm still not sure :)

I left a comment in the other issue you just created about how to improve the tests here.

@ericholscher
Copy link
Member Author

@ericholscher ericholscher commented Dec 10, 2018

I haven't hotfixed it yet, was waiting on review. Will ship it now 👍

@ericholscher ericholscher merged commit e00d928 into master Dec 10, 2018
1 of 3 checks passed
@stsewd stsewd deleted the no-local-branches branch Dec 10, 2018
@agjohnson agjohnson restored the no-local-branches branch Dec 12, 2018
@stsewd stsewd deleted the no-local-branches branch Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants