chuckwagoncomputing / interactive-pinout

Interactive Pinout Generator
MIT License
8 stars 6 forks source link

please fail in case of duplicated pin #1

Closed rusefillc closed 1 year ago

rusefillc commented 1 year ago

Can we please fail for the following yaml with C7 defined twice?

  - pin: C7
    function: WHITE LSU4.9 Heater Negative

  - pin: C7
    function: CAN High
chuckwagoncomputing commented 1 year ago

I can generate a warning if this is the case. Currently, warnings are set to ignored in the workflow. If they are set to skip, there are several other things that will cause pinouts to not be generated for that board. Currently, these are:

Options:

  1. allow more fine-tuned control over what warnings do example options:
    warning-duplicate-pin: skip
    warning-no-cid: false
    warning-no-image: false

    I can, and probably would, program it so that the warning option is still used, but more specific options override it, e.g.:

    warnings: false
    warning-duplicate-pin: skip

    would be equivalent to the above.

  2. Skip boards for all warnings and don't care about boards that don't have images or connector IDs
  3. Check for duplicate pin in ConfigDefinition What is the goal? Is this related to the TS name templating? I threw together a check if this approach interests you.
rusefillc commented 1 year ago

When you say warning do you mean we would have no error return code and no trivial way to fail very visibly?

My preference is a very violent very visible red failure.

chuckwagoncomputing commented 1 year ago

Warnings always print, even when set to false skip skips that board, but doesn't error error fails the entire workflow

If I add fine-toothed warning control, you could error only on duplicates.

If you're wanting to fail the workflow, please note that currently the workflow only runs daily, so a change would be merged already before it resulted in red CI. This would be one advantage of failing in ConfigDefinition, if failing quickly and visibly is what you want.

chuckwagoncomputing commented 1 year ago

I don't really think this is what you're after, but I have now added a notice option for warnings, which shows this on the workflow results page (but still green); image https://github.com/chuckwagoncomputing/rusefi/actions/runs/4214599954

rusefillc commented 1 year ago

No chance for depressing but visible red at all?

chuckwagoncomputing commented 1 year ago

Yep, I can add a warning for duplicates, and the warnings option can be set to error I can (and likely will regardless) also add fine-tune control so other warnings don't cause the workflow to fail.

chuckwagoncomputing commented 1 year ago

This is now available in release 2.1

I think you'll want:

warnings: "false"
warning-dupe: "error"