search

bpmn-io / bpmnlint

Validate BPMN diagrams based on configurable lint rules.
MIT License
119 stars 36 forks source link

feat(rules): implement `global` rule #137

Closed strangelookingnerd closed 4 weeks ago

strangelookingnerd commented 1 month ago

Hey @nikku,

after our short discussion from yesterday I had a go at implementing a rule to validate event references:

I have provided test cases for the rules that demonstrate they actually work as intented. I did not yet provide a documentation because I wanted to make sure that the implementation is correct before documenting it. I was struggeling a bit with validating the usage of a reference and I'm certain that my approach could be improved (see https://github.com/strangelookingnerd/bpmnlint/blob/4e3515167c8d201088cef3a1304bc19be19ec2bb/rules/event-reference.js#L52-L103) Further im not 100% sure that these checks only apply to Error, Escalation and Message but maybe also other elements.

This PR sure still is a little rough on the edges, but I'd be happy to further improve it in order to be merged.

nikku commented 1 month ago

Found one bug related to event definitions without refs, cf. https://github.com/bpmn-io/bpmnlint/pull/137/commits/16c1774429c49ba63fae9d94d102d311e9a2325c.

image

nikku commented 1 month ago

Ensured we stop traversing early (https://github.com/bpmn-io/bpmnlint/pull/137/commits/e239e32d6f4e0f2b91e269575cbf9299888f4555).

strangelookingnerd commented 1 month ago

Found one bug related to event definitions without refs, cf. 16c1774.

Good catch, did not think of that.

nikku commented 1 month ago

Further im not 100% sure that these checks only apply to Error, Escalation and Message but maybe also other elements.

I assume it should also apply for Signal? Or do you see a reason to not apply to it?

strangelookingnerd commented 1 month ago

I assume it should also apply for Signal? Or do you see a reason to not apply to it?

Yes, it should also apply to Signal- oversight on my end. Added it in ca307d3

nikku commented 4 weeks ago

Decision: We'll call this rule global as it handles consistent usage of relevant global elements. We may re-consider the name in the future, once we're smarter.

nikku commented 4 weeks ago

Executed via https://github.com/bpmn-io/bpmnlint/pull/137/commits/c1aadc8c6c7020add90aa33695ff9e43fdef3a6b.

nikku commented 4 weeks ago

Squashed via https://github.com/bpmn-io/bpmnlint/pull/137/commits/f090f86f23fa120719a367b79b70e02b2ca8b589. Thanks!

strangelookingnerd commented 4 weeks ago

Thanks for helping me getting this merged 👍🏼