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.

Sign up for GitHub

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

Jump to bottom

feat(rules): add duplicate-task-headers rule #41

Merged
merged 2 commits into from
Aug 18, 2022
Merged

feat(rules): add duplicate-task-headers rule #41

merged 2 commits into from
Aug 18, 2022

Conversation

pinussilvestrus
Copy link

Related to camunda/linting#4

Adds the duplicate-task-headers rule to check for duplicate zeebe:Header elements (by key).

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Aug 15, 2022
@pinussilvestrus pinussilvestrus mentioned this pull request Aug 15, 2022
Merged
@pinussilvestrus
Copy link
Author

Note that the properties panel is not showing the error yet. Since we display the headers in alphanumeric order and not in the XML order, I find it difficult to determine which header key is causing the error...

@philippfromme
Copy link
Collaborator

I'm reviewing this.

@pinussilvestrus pinussilvestrus changed the title feat(rules): add duplicate-task-headers rule [WIP] feat(rules): add duplicate-task-headers rule Aug 16, 2022
@philippfromme philippfromme force-pushed the duplicate-headers branch 5 times, most recently from 460e30d to 3795971 Compare August 16, 2022 16:51
@philippfromme
Copy link
Collaborator

I made a proposal to refactor this before we merge it (cf. 3795971). @pinussilvestrus will have a look at the proposal and we'll squash before merging.

@nikku nikku added the in progress Currently worked on label Aug 17, 2022 — with bpmn-io-tasks
@nikku nikku removed the needs review Review pending label Aug 17, 2022
pinussilvestrus
pinussilvestrus commented Aug 18, 2022
View reviewed changes
Copy link
Author

@pinussilvestrus pinussilvestrus left a comment

Choose a reason for hiding this comment

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

Works like a charm, nice work 👍 I will clean up the commit history and I think we can go for it 👍

@pinussilvestrus pinussilvestrus changed the title [WIP] feat(rules): add duplicate-task-headers rule feat(rules): add duplicate-task-headers rule Aug 18, 2022
@pinussilvestrus pinussilvestrus added needs review Review pending and removed in progress Currently worked on labels Aug 18, 2022
@pinussilvestrus
Copy link
Author

I updated the PR so I think we can merge it now 👍

philippfromme
philippfromme approved these changes Aug 18, 2022
View reviewed changes
Copy link
Collaborator

@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.

✅

@philippfromme philippfromme merged commit 7e70764 into main Aug 18, 2022
@philippfromme philippfromme deleted the duplicate-headers branch August 18, 2022 11:18
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philippfromme philippfromme philippfromme approved these changes

@nikku nikku Awaiting requested review from nikku

Assignees

@pinussilvestrus pinussilvestrus

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pinussilvestrus @philippfromme @nikku