Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add Linter element-templates linter plugin #3673

Merged
merged 4 commits into from
Jul 25, 2023

Conversation

marstamm
Copy link
Member

@marstamm marstamm commented Jun 16, 2023

Recording 2023-07-11 at 09 18 40
closes #3357

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Jun 16, 2023
@marstamm marstamm force-pushed the 3357-integrate-linter branch 4 times, most recently from 3da43f0 to 0097386 Compare July 12, 2023 07:44
@marstamm marstamm requested review from a team, philippfromme and smbea and removed request for a team July 12, 2023 07:57
@marstamm marstamm marked this pull request as ready for review July 12, 2023 07:57
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Jul 12, 2023
@marstamm
Copy link
Member Author

Pinging @philippfromme as you already did the review on bpmn-io/bpmn-js-element-templates#3

@marstamm marstamm force-pushed the 3357-integrate-linter branch 2 times, most recently from fb280b4 to 7a71714 Compare July 25, 2023 07:16
@@ -194,10 +197,16 @@ export default class TabsProvider {
action: 'create-cloud-bpmn-diagram'
} ];
},
getLinter(plugins) {
async getLinter(plugins = [], tab, app) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too fond of the idea of passing the entire app here. Might as well get the tab from the app then. Usually we only pass the options (e.g. getInitialContents(options)) or the file (e.g. canOpen(file)). I know we need to get hold of the templates, though. 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could limit it to getConfig so we don't pass the whole app

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cf. ae885ef

@philippfromme philippfromme self-requested a review July 25, 2023 15:18
Copy link
Contributor

@philippfromme philippfromme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marstamm marstamm merged commit e4bc84d into develop Jul 25, 2023
9 checks passed
@marstamm marstamm deleted the 3357-integrate-linter branch July 25, 2023 17:04
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show element template errors in the error panel
2 participants