ale icon indicating copy to clipboard operation
ale copied to clipboard

fix(biome): run all enabled biome fixers

Open redbmk opened this issue 1 year ago • 6 comments

  • based on biome config, will format, lint, and/or sort imports
  • adds variable to apply unsafe fixes, disabled by default

fixes: #4754

redbmk avatar Apr 24 '24 22:04 redbmk

@w0rp I have a few other fixes queued up for biome, I just need to make sure I have tests for them and everything. Should I make it all part of one PR or keep them more atomic with separate PRs?

redbmk avatar Apr 29 '24 21:04 redbmk

@redbmk I highly doubt that waiting this long for a response is worth it. Maybe you can just handle it how you like.

I appreciate your work!

Spixmaster avatar May 08 '24 10:05 Spixmaster

Smaller self-contained PRs are easier to review given limited time maintainers have. Also easier to revert in case something breaks.

hsanson avatar May 08 '24 11:05 hsanson

There is a linter issue that causes the test to fail. After that is fixed I can merge this:

./autoload/ale/fixers/biome.vim:7 Blank line required before `if`.

hsanson avatar May 09 '24 00:05 hsanson

There is a linter issue that causes the test to fail. After that is fixed I can merge this:

./autoload/ale/fixers/biome.vim:7 Blank line required before `if`.

@hsanson Cool I just pushed up a fix that included your oneliner suggestion. That's a lot cleaner, thanks! I checked that linters are passing locally now

redbmk avatar May 09 '24 04:05 redbmk

Not sure if it would be an issue in the test runners but I noticed if I run the linter and then fixer tests I get an error. I think if it runs fixers first then linters, it wouldn't show up, but the fix I just pushed lets it work either way.

redbmk avatar May 13 '24 16:05 redbmk

What is the issue with the commits? Why do not the tests finish?

I would like to inspect the output but can only call the one of "continuous-integration/appveyor/pr".

Edit: I just saw the comment of @redbmk, https://github.com/dense-analysis/ale/pull/4775#issuecomment-2180645822.

Spixmaster avatar Jun 20 '24 15:06 Spixmaster

@redbmk if you could fix the conflict I can merge this.

hsanson avatar Jun 20 '24 23:06 hsanson

LGTM, thanks

looks like I need to fix merge conflicts with the other one now. I'll try to do that asap

redbmk avatar Jun 20 '24 23:06 redbmk

@hsanson should be good now

redbmk avatar Jun 21 '24 00:06 redbmk