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.

Sign up for GitHub

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

Jump to bottom

[READY] Only add the necessary directories to Python path #1124

Merged
merged 1 commit into from
Oct 18, 2018
Merged

[READY] Only add the necessary directories to Python path #1124

merged 1 commit into from
Oct 18, 2018
+60 −192

Conversation

micbou
Copy link
Collaborator

@micbou micbou commented Oct 14, 2018 •
edited by Valloric
Loading

See PR ycm-core/YouCompleteMe#3163.

This change is Reviewable

@micbou micbou changed the title [READY] Only add the necessary directories to Python path [WIP] Only add the necessary directories to Python path Oct 14, 2018
@micbou micbou changed the title [WIP] Only add the necessary directories to Python path [READY] Only add the necessary directories to Python path Oct 14, 2018
bstaletic
bstaletic approved these changes Oct 14, 2018
View reviewed changes
Copy link
Collaborator

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since a bunch of tests got removed in server_utils_tests.py, do we need anything to replace them? I never thought those tests did anything except annoy us when we change submodules, so :lgtm:.

Reviewed 3 of 3 files at r1.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale)

@codecov
Copy link

codecov bot commented Oct 14, 2018 •
edited
Loading

Codecov Report

Merging #1124 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1124      +/-   ##
==========================================
+ Coverage   97.64%   97.67%   +0.02%     
==========================================
  Files          90       90              
  Lines        7037     7039       +2     
==========================================
+ Hits         6871     6875       +4     
+ Misses        166      164       -2

micbou
micbou commented Oct 15, 2018
View reviewed changes
Copy link
Collaborator Author

@micbou micbou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since a bunch of tests got removed in server_utils_tests.py, do we need anything to replace them? I never thought those tests did anything except annoy us when we change submodules, so :lgtm:.

I don't think we need to replace them since this part of the code is already covered by the other tests. And yes, those tests were a nuisance. I wonder who added them. Can't be me, right?

Reviewed 3 of 3 files at r1.
Reviewable status: 1 of 2 LGTMs obtained

Valloric
Valloric approved these changes Oct 18, 2018
View reviewed changes
Copy link
Member

@Valloric Valloric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

:lgtm:

@zzbot r+

Reviewable status: 1 of 2 LGTMs obtained (and 1 stale)

@bstaletic
Copy link
Collaborator

@zzbot r+

@zzbot
Copy link
Contributor

zzbot commented Oct 18, 2018

📌 Commit dcf686c has been approved by bstaletic

@zzbot
Copy link
Contributor

zzbot commented Oct 18, 2018

⌛ Testing commit dcf686c with merge 600f54d...

zzbot added a commit that referenced this pull request Oct 18, 2018
@zzbot
Auto merge of #1124 - micbou:python-path, r=bstaletic
600f54d 
[READY] Only add the necessary directories to Python path

See PR ycm-core/YouCompleteMe#3163.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/1124)
<!-- Reviewable:end -->
@zzbot
Copy link
Contributor

zzbot commented Oct 18, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: bstaletic
Pushing 600f54d to master...

@zzbot zzbot merged commit dcf686c into ycm-core:master Oct 18, 2018
@zzbot zzbot mentioned this pull request Oct 18, 2018
Merged
@micbou micbou deleted the python-path branch November 17, 2018 00:09
@micbou micbou mentioned this pull request Nov 17, 2018
Merged
zzbot added a commit to ycm-core/YouCompleteMe that referenced this pull request Nov 18, 2018
@zzbot
Auto merge of #3174 - micbou:clang-fixit-completion, r=bstaletic
ff0a62b 
[READY] Update ycmd

Include the following changes:

 - PR ycm-core/ycmd#1080: replace Boost canonical function with our own implementation;
 - PR ycm-core/ycmd#1104: improve filename completer;
 - PR ycm-core/ycmd#1121: support completion FixIts for C-family languages;
 - PR ycm-core/ycmd#1122: update jdt.ls to 0.26.0;
 - PR ycm-core/ycmd#1123: install fixed version of TypeScript in third-party folder;
 - PR ycm-core/ycmd#1124: only add the necessary directories to Python path.

Fixes #3173.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/3174)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Valloric Valloric Valloric approved these changes

@bstaletic bstaletic bstaletic approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@micbou @bstaletic @zzbot @Valloric