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

Suppress stderr in call to ruff and add ruff formatter #333

Merged
merged 4 commits into from
Jul 22, 2024

Conversation

felix-cw
Copy link
Contributor

This PR adds the fix I suggested in #331 to ensure compatibility with ruff 0.1.0+. I've confirmed it works with version 0.1.15.

I also added the ruff formatter itself, as a substitute for black, called "ruffformat". The formatter simply called "ruff" is for applying automatic fixes for diagnostics. For example, isort-style import sorting.

@MordorianGuy
Copy link

Would it be more adaptive to initialise the command depending on the ruff version which may be acquired via subprocess.run?
By the way, could you, please, elucidate a little how to install the package during development? When I install the released version of package via pip into a hollow environment (mamba create -n test pip) it installs without any problem. But when I am trying to install it either from my fork repository (via git+[link] notation) or directly from a local path (pip install -e [path]) the installation fails.

@felix-cw
Copy link
Contributor Author

Hi. Thanks for the comment. I believe the added flags for the ruff --fix command will work for versions pre 0.1.0 so no change required. Since ruff formatting was added more recently, the ruff command would fail for previous versions that do not support it. Since using these formatters is opt-in, we might expect someone who turns it on to have a compatible ruff version installed. Is there a way to warn the user they need to update their ruff version to use the feature?

On the installation, honestly I just edited the files in my conda env and transcribed the changes here. I know that's not right!
The doc site has a page about installing it for development but I haven't tried it yet.

@krassowski
Copy link
Contributor

@felix-cw would you like to rebase this PR to make sure the tests pass on CI?

@krassowski krassowski added the bug Something isn't working label Jul 19, 2024
@krassowski
Copy link
Contributor

Also, adding a test here would make it easier to merge it :)

@felix-cw
Copy link
Contributor Author

I added a few tests for the ruff and ruffformat functionality.

Copy link
Contributor

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

Thank you @felix-cw!

@krassowski krassowski merged commit 07f0478 into jupyterlab-contrib:master Jul 22, 2024
5 checks passed
@krassowski
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants