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

[READY] Clean C# inserting namespace code #2650

Merged
merged 1 commit into from
May 20, 2017

Conversation

micbou
Copy link
Collaborator

@micbou micbou commented May 17, 2017

Now that we require at least Vim 7.4.1578, we can remove the code that automatically insert namespaces in C# for Vim versions prior to 7.4.774.


This change is Reviewable

Remove code that automatically insert namespaces in C# for Vim versions prior
to 7.4.774.
@micbou micbou force-pushed the clean-cs-inserting-namespace-code branch from 856fbd3 to 1fc6b4a Compare May 17, 2017 17:44
@codecov-io
Copy link

codecov-io commented May 17, 2017

Codecov Report

Merging #2650 into master will increase coverage by 0.08%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2650      +/-   ##
==========================================
+ Coverage   88.48%   88.57%   +0.08%     
==========================================
  Files          19       19              
  Lines        1946     1908      -38     
==========================================
- Hits         1722     1690      -32     
+ Misses        224      218       -6

@bstaletic
Copy link
Collaborator

:lgtm:


Review status: 0 of 3 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@Valloric
Copy link
Member

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion.


python/ycm/vimsupport.py, line 87 at r1 (raw file):

# Expects version_string in 'MAJOR.MINOR.PATCH' format, e.g. '7.4.301'
def VimVersionAtLeast( version_string ):

This function looks useful to have. We might not need it right now, but I can easily see us needing it again, no?

I don't have a strong opinion though.


Comments from Reviewable

@bstaletic
Copy link
Collaborator

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


python/ycm/vimsupport.py, line 87 at r1 (raw file):

Previously, Valloric (Val Markovic) wrote…

This function looks useful to have. We might not need it right now, but I can easily see us needing it again, no?

I don't have a strong opinion though.

I would go with removing it. I'm one of those who think unneeded code should be remved from the codebase aggressivly. In case something from the past is needed, well we all know how to use git, don't we?


Comments from Reviewable

@vheon
Copy link
Contributor

vheon commented May 19, 2017

:lgtm:


Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


python/ycm/vimsupport.py, line 87 at r1 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

I would go with removing it. I'm one of those who think unneeded code should be remved from the codebase aggressivly. In case something from the past is needed, well we all know how to use git, don't we?

I think I'm with @bstaletic here, specially since this function is also not covered by any test.


Comments from Reviewable

@micbou
Copy link
Collaborator Author

micbou commented May 19, 2017

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


python/ycm/vimsupport.py, line 87 at r1 (raw file):

Previously, vheon (Andrea Cedraro) wrote…

I think I'm with @bstaletic here, specially since this function is also not covered by any test.

Also, not having this function means that we don't have code specific to some versions of Vim which is rather a good thing.


Comments from Reviewable

@Valloric
Copy link
Member

Lots of LG's, let's go!

@zzbot r=bstaletic


Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


python/ycm/vimsupport.py, line 87 at r1 (raw file):

Previously, micbou wrote…

Also, not having this function means that we don't have code specific to some versions of Vim which is rather a good thing.

You've convinced me, let's remove it. :)


Comments from Reviewable

@zzbot
Copy link
Contributor

zzbot commented May 20, 2017

📌 Commit 1fc6b4a has been approved by bstaletic

@zzbot
Copy link
Contributor

zzbot commented May 20, 2017

⌛ Testing commit 1fc6b4a with merge 263bd88...

zzbot added a commit that referenced this pull request May 20, 2017
…aletic

[READY] Clean C# inserting namespace code

Now that we require at least Vim 7.4.1578, we can remove the code that automatically insert namespaces in C# for Vim versions prior to 7.4.774.

<!-- 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/2650)
<!-- Reviewable:end -->
@zzbot
Copy link
Contributor

zzbot commented May 20, 2017

☀️ Test successful - status-travis
Approved by: bstaletic
Pushing 263bd88 to master...

@zzbot zzbot merged commit 1fc6b4a into ycm-core:master May 20, 2017
@micbou micbou deleted the clean-cs-inserting-namespace-code branch June 19, 2017 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants