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

Build: stop failing the build on eslint errors #1594

Merged
merged 2 commits into from
Sep 27, 2019

Conversation

dmsnell
Copy link
Contributor

@dmsnell dmsnell commented Sep 24, 2019

eslint provides helpful warnings that prevent bugs in our software.
However by running it as a precondition for the build we introduce
artificial obstacles to quick testing and iteration. For instance, we
will fail to build the app if we have an unused variable even though
there's no reason it would break the build.

In this change we're removing the precondition for passing eslint
before building the app. This should allow us to more rapidly made small
debugging/testing adjustments while working on fixing or altering the
app.

eslint still belongs in the process and we might follow-up on this PR
by introducing it as a git hook on precommit, a more appropriate
place for the safety check. For now though I am foregoing that work in
order to remove what has been a persistent frustration while working on
the app.

This should have no visual or functional change on the app. This should
only impact the build of the application while developing and debugging.

Copy link
Contributor

@beaucollins beaucollins left a comment

Choose a reason for hiding this comment

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

Makes sense, I have this commented out on my build right now.

@beaucollins
Copy link
Contributor

Related #1596

@belcherj
Copy link
Contributor

eslint still belongs in the process and we might follow-up on this PR
by introducing it as a git hook on precommit, a more appropriate
place for the safety check. For now though I am foregoing that work in
order to remove what has been a persistent frustration while working on
the app.

This is on my list of wants. Seems like an easy GitHub Action. I will follow up with a precommit hook as I've written that code a number of times. Probably won't get to the server check for a while.

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

Linters should provide helpful information but generally stay out of the way 👍

enforce: 'pre',
use: [
{
loader: 'eslint-loader',
Copy link
Member

Choose a reason for hiding this comment

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

This is likely a dependency that can be removed as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I'll update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed dependency in 065321e

`eslint` provides helpful warnings that prevent bugs in our software.
However by running it as a precondition for the build we introduce
artificial obstacles to quick testing and iteration. For instance, we
will fail to build the app if we have an unused variable even though
there's no reason it would break the build.

In this change we're removing the precondition for passing `eslint`
before building the app. This should allow us to more rapidly made small
debugging/testing adjustments while working on fixing or altering the
app.

`eslint` still belongs in the process and we might follow-up on this PR
by introducing it as a `git` hook on `precommit`, a more appropriate
place for the safety check. For now though I am foregoing that work in
order to remove what has been a persistent frustration while working on
the app.
@dmsnell dmsnell force-pushed the build/stop-failing-build-on-eslint-error branch from c30225b to 065321e Compare September 27, 2019 02:25
@dmsnell dmsnell merged commit 31e6bc7 into develop Sep 27, 2019
@dmsnell dmsnell deleted the build/stop-failing-build-on-eslint-error branch September 27, 2019 02:48
@beaucollins
Copy link
Contributor

@dmsnell dmsnell added this to the 1.10 milestone Oct 1, 2019
@loremattei loremattei modified the milestones: 1.10, 1.9 Oct 7, 2019
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.

None yet

5 participants