Exit with status code 1 on lumo warnings

nervous-systems / serverless-cljs-plugin

Serverless plugin for Clojurescript deployment w/ cljs-lambda

The Unlicense

75 stars 10 forks source link

Exit with status code 1 on lumo warnings #30

Closed arichiardi closed 6 years ago

arichiardi commented 6 years ago

Logs are hidden during serverless compilation, effectively making impossible to see compilation warnings. Now the serverless-cljs-plugin allows to specify a --warningExit or cljsWarningExit option in order to error out at the first warning.

Warnings are configurable through the :warnings compiler option.

moea commented 6 years ago

@arichiardi I'm not sure about this behaviour - it's unlikely to me that in all cases :warnings is sufficiently expressive to rely on for deciding whether a compilation was successful. I'd much prefer just logging the compiler output, and suggesting the users do SLS_DEBUG='*' sls deploy. I would also hope that occasionally users compile their code outside of the deploy cycle, to run tests or start a REPL, so I don't think the plugin needs to try and be definitive about detecting compiler warnings.

arichiardi commented 6 years ago

Maybe I did not explain this well...Basically without this the code is compiled and deployed as if correct, and you don't see WARNING logging anyways, even with the environment var set, because we are not using the serverless logger in the lumo build.

So you effectively deploy with warnings (that 99% of the times are uncaught mistakes) and then it fails at runtime.

Maybe we can hide it behind an option (and I would like at some point to add one directly in lumo)?

This blog post does the same btw. On a side note, I never understood why the compiler emits warnings instead of errors, probably something to do with the Google Closure Compiler.

moea commented 6 years ago

@arichiardi I understood what you meant - my points were just that:

Many warnings are spurious, so the code may be correct
It may not be possible for a user to perfectly exclude the spurious warnings without just disabling all warnings
If the lumo script logs the compiler output, the viewer can see this by setting SLS_DEBUG='*' - it's up to them whether they see or act on the warnings.

arichiardi commented 6 years ago

Ok so feature flag then? I am the kind of user that prefers to have explicit errors :) This feature can be also used in CI environment where you want to stop when whatever thing does not match expectations (like the blog post above)

moea commented 6 years ago

A flag is fine, yeah!

arichiardi commented 6 years ago

Pushed a new commit, let me know which one you would like to merge first and I can rebase the other one ok? (well, I guess I will see it :smile:)

arichiardi commented 6 years ago

Ok I addressed the comment and also revamped the README a bit (hopefully for the better)