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

Rename to pylsp prefix #11

Merged
merged 8 commits into from
Jul 3, 2021
Merged

Rename to pylsp prefix #11

merged 8 commits into from
Jul 3, 2021

Conversation

Richardk2n
Copy link
Member

No description provided.

@Richardk2n
Copy link
Member Author

What do you think of this @haplo (I am asking, because you seem to have more experience).
It will break old config files, but it will probably (help) fix issues like #7 and avoid confusion about potential causes for errors as in #9

@haplo
Copy link

haplo commented Jun 28, 2021

Breaking changes would be bad for existing users, but good for new users. If we could find a way to ease the transition that would be best. An idea is to support multiple configuration namespaces (like mypy-ls, mypy_ls, pylsp_mypy), but deciding on a canonical one. Then try reading from the canonical one first, if not available try the other ones and raise a DeprecationWarning if any of those are found. That would hint existing users to update their configuration. The DeprecationWarning can be removed and an error raised instead in the future once everybody has had time to update.

About PyPI, I see you already published pylsp-mypy. You should probably release mypy-ls with a big DEPRECATED warning on top telling users to switch to pylsp-mypy.

Also changed the line length and made the config discovery less dependet on string manipulation.
Added deprecation warnings for old namespaces
@Richardk2n
Copy link
Member Author

I added deprecation warnings and intend to push an update to mypy-ls to depend on pylsp-mypy and contain nothing else, so existing users get upgraded automatically. What is left is renaming the GitHub repo and changing the link in setup.cfg

@Richardk2n Richardk2n merged commit 997c318 into master Jul 3, 2021
@Richardk2n Richardk2n deleted the rename branch July 3, 2021 17:57
@rchl
Copy link

rchl commented Jul 3, 2021

Did it actually address #7? I think it will still be the case that the setting for enabling will have to be called pylsp-mypy.enabled but other settings will have to be called pylsp_mypy.* (note the dash vs. underscore).

@rchl
Copy link

rchl commented Jul 3, 2021

The setting for enabling is somehow automatically inferred by pylsp, I believe, and I'm not sure if it's even possible to change it (couldn't figure out after quick look at least). So changing the naming of pylsp_mypy settings might be an option instead.

@rchl
Copy link

rchl commented Jul 3, 2021

No, sorry, looking at the diff it looks like this was actually addressed. It should use underscore consistently now. Thanks!

@Richardk2n
Copy link
Member Author

The setting for enabling is somehow automatically inferred by pylsp, I believe, and I'm not sure if it's even possible to change it (couldn't figure out after quick look at least). So changing the naming of pylsp_mypy settings might be an option instead.

I am assuming, the name is automatically inferred from either the package name which has an underscore or the entry point which also has an entry point, but i don't know to be honest.

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.

3 participants