polyfy / polylith

A tool used to develop Polylith based architectures in Clojure.
Eclipse Public License 1.0
523 stars 48 forks source link

Enable the option to treat validation warnings as errors #479

Closed tengstrand closed 2 months ago

tengstrand commented 4 months ago

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}
seancorfield commented 4 months ago

There are two separate issues here:

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 commented 2 months ago

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

tengstrand commented 2 months ago

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