Add explanation to no-inclusive-gateway rule

bpmn-io / bpmnlint

Validate BPMN diagrams based on configurable lint rules.

MIT License

121 stars 36 forks source link

Add explanation to no-inclusive-gateway rule #45

Closed barmac closed 3 years ago

barmac commented 3 years ago

Related to #41 Closes #44

Definition of Done

[ ] corresponds to the design principles
[ ] corresponds to the code standards
[ ] passes Continuous Integration checks
[ ] available as feature branch on GitHub
- [ ] contains the cleaned up commit history
- [ ] commit messages satisfy our commit message guidelines
- [ ] a single commit closes the issue via Closes #issuenr

barmac commented 3 years ago

The build failed on Node 15 even though I have not changed code.

pinussilvestrus commented 3 years ago

Interesting that it is failing because of one single new line, cf. https://travis-ci.com/github/bpmn-io/bpmnlint/jobs/404323815#L550

actual

\n\n/home/travis/build/bpmn-io/bpmnlint/test/integration/cli/diagram-broken.bpmn\n    error  Parse error: failed to parse document as <bpmn:Definitions>\n\n✖ 1 problem (1 error, 0 warnings)\n\n

expect

\n\n/home/travis/build/bpmn-io/bpmnlint/test/integration/cli/diagram-broken.bpmn\n    error  Parse error: failed to parse document as <bpmn:Definitions>\n\n✖ 1 problem (1 error, 0 warnings)\n

See the additional newline at the end.

nikku commented 3 years ago

Long story short, we should fix our CI builds to not use latest Node as too many magic things can happen with pre-stable node versions (i.e. 11, 13, 15, ...).

Fixed this one via https://github.com/bpmn-io/bpmnlint/pull/45/commits/3eb1acf5bc5611b3fe334b8132705064da2033e2.

barmac commented 3 years ago

Thanks. How do you like the PR?

nikku commented 3 years ago

Generally, as this is a matter of style, we do not provide rationale for these rules, similar to how I thought eslint does it.

In some cases however, eslint provides strong rationale for why something should not be done, example: https://eslint.org/docs/rules/for-direction#rule-details

So I'd be fine adding an explaination here.

barmac commented 3 years ago

I think that as long as rules don't lead to confusion on forums/issues, it's OK to assume they are self-explanatory. In this case, however, there have been a GitHub issue and a forum post questioning rule's existence.