Athou / commafeed

Google Reader inspired self-hosted personal RSS reader.
https://www.commafeed.com
Apache License 2.0
2.81k stars 377 forks source link

disable check on build #1568

Closed honnip closed 1 month ago

honnip commented 1 month ago

This makes the build work without biome.

Athou commented 1 month ago

I like that the build fails if there are errors in linting/formatting. It also helps when reviewing pull requests. Why would you want to disable it?

honnip commented 1 month ago

And it seems a bit odd to me to check it during the build phase 😃

Athou commented 1 month ago

biome is required at build time

In order to be usable by npm run dev and npm run lint, biome needs to be in the devDependencies , which is the case. By being in the devDependencies it's also available at build time, your PR doesn't change that, I'm not sure I understand your point.

If the biome updates and behaves differently, the build may fail

I consider this a feature. Let's say they find a bug in useExhaustiveDependencies that causes the linter to not report all missing dependencies, I'd rather be informed as soon as renovate creates a PR with a fix so that I can take action.

And it seems a bit odd to me to check it during the build phase 😃

I guess the check could be disabled in npm run build and moved to the frontend-maven-plugin in pom.xml, but the end result would be the same, the build would still fail on linter errors. I really never call npm run build directly though, it's always called by maven when the whole project is built during CI.

I'm not sure what you want to accomplish with this PR, are you trying to fix an issue you're having or is this just a matter of opinion? Because if it's the latter, I think we'll have to agree to disagree 😄

honnip commented 1 month ago

I agree with your point. As I'm not familiar with maven, I missed things