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

modify code to get rid of deprecation warnings #6

Merged
merged 3 commits into from
Aug 3, 2020
Merged

modify code to get rid of deprecation warnings #6

merged 3 commits into from
Aug 3, 2020

Conversation

Sawrz
Copy link

@Sawrz Sawrz commented Aug 2, 2020

Hi,
I get some deprecation warnings in the console by installing the mdl linter:

SublimeLinter: WARNING: mdl: `self.get_view_settings()` has been deprecated.  Just use the member `self.settings` which is the same thing.
SublimeLinter: WARNING: mdl: `executable_path` has been deprecated. Just use an ordinary binary name instead. 
SublimeLinter: WARNING: mdl: Implicit appending a filename to `cmd` has been deprecated, add '${temp_file}' explicitly.

So, to get rid of them, I modified the code a bit. It runs fine on my installation. Additionally, I changed the linter class to tackle issue #5. I don't see any benefits of using RubyLinter instead of Linter. However, maybe there are reasons for your choice, but in issue #5, it seems you're open-minded to change that.

Best,
Sandro

@braver
Copy link
Member

braver commented Aug 2, 2020

There is probably no tangible benefit of using RubyLinter. The changes here look simple enough, and good, to me. But I don’t know this linter or how it works. I’m inclined to just merge. Maybe give @kaste a chance to say something about it first.

@kaste
Copy link
Contributor

kaste commented Aug 2, 2020

The intention in #5 and then SublimeLinter/SublimeLinter#1759 is to use the minimal RubyLinter from rubocop. Basically, copy-pasting https://github.com/SublimeLinter/SublimeLinter-rubocop/blob/master/linter.py#L5-L22 to define a local RubyLinter base would do the job. This has the benefit of a) that we have a bigger base install before we actually make the switch, b) this linter here ("mdl") would still be marked as a ruby tool. (For example, we can later still grep RubyLinter against all public linter plugins.)

Otherwise looks good. (Actually scary old plugin code here, I didn't know we still ship the fallback code used here.)

@Sawrz
Copy link
Author

Sawrz commented Aug 3, 2020

I added the requested change. Still, I'm not sure if I got your point @kaste.
a) Could you go more into detail? What do you mean by a bigger base install?
b) So far, I understand you want to mark mdl as a ruby tool. However, isn't it evident by installing the package anyway? I mean, I can't use the package unless I install mdl for which I need Ruby. Additionally, adding another class adds another layer of complexity, which might or might not be necessary.

We can later still grep RubyLinter against all public linter plugins.

What do you mean by that?

@kaste
Copy link
Contributor

kaste commented Aug 3, 2020

If we ship this RubyLinter here we have more users using this abstraction, getting more confidence that it is the correct abstraction or a better one than the one we have in SublimeLinter core right now. The plan is then to copy paste this to the core, and remove it everywhere else.

To manage all the public registered SublimeLinter plugins I just grep for RubyLinter t.i. I do a normal source code search, to see all ruby linters; or I grep a specific API/feature to see if we can deprecate or remove something.

@kaste kaste merged commit 4705269 into SublimeLinter:master Aug 3, 2020
@kaste
Copy link
Contributor

kaste commented Aug 3, 2020

Let's see if this works out well and ship it.

@kaste
Copy link
Contributor

kaste commented Aug 3, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants