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

Enable the option to treat validation warnings as errors #479

Closed
tengstrand opened this issue Jun 30, 2024 · 3 comments
Closed

Enable the option to treat validation warnings as errors #479

tengstrand opened this issue Jun 30, 2024 · 3 comments

Comments

@tengstrand
Copy link
Collaborator

tengstrand commented Jun 30, 2024

Today, the test command will not run if we have errors in the workspace, and if we run the check command it will return 0 (zero) only if there are no errors in the workspace.

If you prefer a stricter approach and want to avoid warnings in your workspace, it would be beneficial to have the option to disallow warnings.

The suggestion is that we add this key to workspace.edn:

{
  ...
  :allow-warnings true
}

If set to true then we get today's behavior, but if set to false then some commands, and the public API, will treat warnings more strictly.

check

:allow-warnings is set to false:
Warnings will now be treated as errors, causing them to be displayed and returning the corresponding warning code (2xx), which can be used in scripts.

:allow-warnings is set to true:
We get the today's behavior that displays OK and returns 0 (zero) if there are no errors, but allows warnings.

test

:allow-warnings is set to false:
If warnings are present, they will be displayed, execution will stop, and the warning code will be returned, mirroring the behavior for errors.

:allow-warnings is set to true:
We get today's behavior where the execution stops only if we have errors (warnings are ignored).

check - public API

The check command in the public API, will also be affected.

:allow-warnings is set to false:
Will treat warnings in the same ways as errors, for example:

{:ok? false
 :error-messages [{:type "warning",
                   :code 202,
                   :message "Missing paths was found"}]}

:allow-warnings is set to true:
The above example will work as today and notify that the workspace is okay:

{:ok? true
 :error-messages []}

test - public API

:allow-warnings is set to false:
The test command in the public API will treat the workspace as invalid if there are any warnings, and it will stop the execution without running any tests.

{:ok? false}

:allow-warnings is set to false:
Will have today's behavior where it treats the workspace as invalid if it has errors, and stop the execution, but will allow warnings. The error message is printed, returning false if errors are present and true otherwise:

{:ok? true}
@tengstrand tengstrand changed the title Make it configurable, to treat validation errors as errors Make it possible to treat validation warnings as errors Jun 30, 2024
@tengstrand tengstrand changed the title Make it possible to treat validation warnings as errors Enable the option to treat validation warnings as errors. Jun 30, 2024
@tengstrand tengstrand changed the title Enable the option to treat validation warnings as errors. Enable the option to treat validation warnings as errors Jun 30, 2024
@seancorfield
Copy link
Contributor

There are two separate issues here:

  • should check and test display warnings? (and should the check public API return warnings?)
  • should warnings be treated as errors?

One Boolean setting is not sufficient for this.

Currently, the only way to see warnings at all, is via the info command -- I was surprised to learn this because I assumed check and test always displayed warnings (even if warnings did not affect the exit code / ok? result).

At a minimum, I want a way to have check, test, and the public check API to display/return warnings without affecting the exit status / ok? result. I don't care whether that is the default behavior or has to be explicitly requested.

I, personally, do not want warnings treated as errors -- that is a completely separate issue as far as I'm concerned and should be opt-in behavior separate from whether check and test` display warnings.

@tengstrand
Copy link
Collaborator Author

I created issue #509 instead. I don't think we need to treat warnings as errors. @seancorfield

@tengstrand
Copy link
Collaborator Author

To be clear (if I said something else) the check and test commands already include warnings.

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

No branches or pull requests

2 participants