feat(cloud-templates): add linter plugin #3
Conversation
marstamm
requested review from
philippfromme and
smbea
and removed request for
a team
| node.id, | ||
| `${property.label} ${firstLetterToLowerCase(error)}`, | ||
| { | ||
| entryIds: [ getEntryId(property, template) ], |
There was a problem hiding this comment.
The structure of a report should be
{
"propertiesPanel": {
"entryId": "foo"
}
}according to @camunda/linting. I noticed that this structure is also assumed here. I'd like to change this so we have one structure instead of two.
There was a problem hiding this comment.
I initially decided against this as one report can refer to multiple entries in the properties panel (eg. duplicated property error), which is why entryIds is an array.
The entryId set in the report which is used to focus is just the first from the array. Just adding a single entryId would not allow linter plugins to reference multiple property panel entries.
We could nest the entryIds option into the propertiesPanel and keep it side-by-side with the entryId:
propertiesPanel: {
entryIds: ["foo", "bar"]
entryId: "foo"
}
Alternatively, we can always keep the full list of entryIds for the properties panel and select the first one when we fire the event here. What do you think?
There was a problem hiding this comment.
For 2️⃣ , I created a draft PR in linting with the proposed changes: camunda/linting#63
There was a problem hiding this comment.
I adjusted the PR to aligned with 2️⃣ as of 201c4ec. Together with camunda/linting#63, this will align our report structure and allows linter plugins to supply more than one entryId
bpmn-io-tasks
bot
added
in progress
* add bpmnlint * add assert * update to @camunda/[email protected] * update to bpmn-js-properties-panel@2
marstamm
added
needs review
| }; | ||
|
|
||
|
|
||
| export const ElementTemplateLinterPlugin = function(templates) { |
There was a problem hiding this comment.
It took me some time to understand what's going on here. So basically we have a bpmnlint plugin buried in the the bpmn-js-element-templates project that is hidden in a file called Linter which isn't a linter but exports a rule and also a bpmnlint plugin. Maybe we can at least rename the file so it's less confusing.
There was a problem hiding this comment.
Agreed, let's make the name clearer --> ba1d96e
The rule is only exported for the testing and not exposed in the final bundle. There, only the Plugin is exported:
| // We use the ElementTemplates Module without the required bpmn-js modules | ||
| // As we only use it to facilitate template ID and version lookup, | ||
| // access to commandstack etc. is not required | ||
| const elementTemplates = new ElementTemplates(); |
There was a problem hiding this comment.
What can go wrong in the future here? 🙈
|
Nicely done 🏅 |
related to camunda/camunda-modeler#3357