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

Jump to bottom

Resolve Explicitly Exported Plugin Rules #99

Merged
merged 4 commits into from Jun 12, 2023
Merged

Resolve Explicitly Exported Plugin Rules #99

merged 4 commits into from Jun 12, 2023

Conversation

philippfromme
Copy link
Contributor

@philippfromme philippfromme commented Mar 16, 2023 •
edited by nikku

Gives plugin authors the freedom to choose their own plugin structure (cf. camunda/bpmnlint-plugin-camunda-compat#87).

Example:

module.exports = {
  rules: {
    baz: './foo/bar/baz'
  }
};

Similar to ESLint, just supporting path references over require calls; require calls cannot easily be bundled for the browser, cf. #99 (comment), #99 (comment).

Related to camunda/bpmnlint-plugin-camunda-compat#87

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Mar 16, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this pull request Mar 16, 2023
@philippfromme
chore: separate C7 and C8 rules
4ee2305 
Requires bpmn-io/bpmnlint#99
@philippfromme philippfromme mentioned this pull request Mar 16, 2023
Merged
nikku
nikku requested changes Mar 16, 2023
View reviewed changes
Copy link
Member

@nikku nikku left a comment

Choose a reason for hiding this comment

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

Nice stuff 🎉.

Let's add a few more tests and then we can get this merged.

@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Mar 16, 2023
@nikku nikku marked this pull request as draft March 21, 2023 15:25
@philippfromme philippfromme force-pushed the resolve-explicitly-exported-plugin-rules branch 3 times, most recently from e24d7cf to 2f752a3 Compare March 29, 2023 06:46
@philippfromme philippfromme marked this pull request as ready for review March 29, 2023 06:59
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Mar 29, 2023
@philippfromme philippfromme force-pushed the resolve-explicitly-exported-plugin-rules branch from 2f752a3 to 1bdb23c Compare March 29, 2023 07:05
@nikku nikku self-requested a review March 29, 2023 12:29
nikku
nikku previously requested changes Mar 29, 2023
View reviewed changes
Copy link
Member

@nikku nikku left a comment

Choose a reason for hiding this comment

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

Eagerly waiting for #99 (comment).

@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Mar 29, 2023
@philippfromme philippfromme force-pushed the resolve-explicitly-exported-plugin-rules branch from 1bdb23c to c19c638 Compare March 29, 2023 12:33
@philippfromme
Copy link
Contributor Author

@nikku I separated the changes into one commit that refactors the existing tests before the new feature is added by a second commit. Hopefully, that's easier to review.

@philippfromme
Copy link
Contributor Author

@nikku I added a commit testing the compilation of local configs. It was indeed missing. Feel free to squash.

@nikku nikku dismissed their stale review June 8, 2023 13:20

Accounted for.

@nikku nikku marked this pull request as ready for review June 8, 2023 13:20
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Jun 8, 2023
@nikku
Copy link
Member

nikku commented Jun 8, 2023

@philippfromme This looks simple enough now. Added the discussed improvements:

Check it out, give me a 👍 and I'll do a final PR cleanup before merging it.

@nikku nikku requested a review from barmac June 8, 2023 13:22
@philippfromme
Copy link
Contributor Author

Check it out, give me a 👍 and I'll do a final PR cleanup before merging it.

Let me have a quick look!

@nikku nikku requested review from marstamm and smbea June 8, 2023 13:22
@nikku
Copy link
Member

nikku commented Jun 8, 2023

Assigning @barmac and @smbea for review (non-blocking). Just FYI. :)

philippfromme
philippfromme commented Jun 8, 2023
View reviewed changes
Copy link
Contributor Author

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

Looks good. Let's 🚢 it!

@nikku nikku force-pushed the resolve-explicitly-exported-plugin-rules branch 3 times, most recently from 744ac4a to a20a257 Compare June 11, 2023 17:41
@nikku
Copy link
Member

nikku commented Jun 11, 2023

@philippfromme a20a257 should wrap up this topic.

philippfromme and others added 4 commits June 12, 2023 08:43
@philippfromme @nikku
test: refactor node-resolver tests
2dbfc4d
@philippfromme @nikku
feat: resolve path exported rules
b103f02 
This adds the ability to ship rules in a non-default location, exported
through the plug-in main entry:

```javascript
module.exports = {
  rules: {
    'foo': 'src/foo',
    'bar': 'src/bar'
  }
};
```

The location will be interpreted local to the package root.
@nikku
feat: resolve custom rule path relative to entry point
be797f2
@nikku
chore: normalize paths, posix style
eb7445a
@nikku nikku force-pushed the resolve-explicitly-exported-plugin-rules branch from 0c477d9 to eb7445a Compare June 12, 2023 06:43
@philippfromme
Copy link
Contributor Author

I verified it works with bpmnlint-plugin-camunda-compat.

@philippfromme philippfromme merged commit 9773c22 into master Jun 12, 2023
16 checks passed
@philippfromme philippfromme deleted the resolve-explicitly-exported-plugin-rules branch June 12, 2023 13:27
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Jun 12, 2023
philippfromme added a commit to camunda/bpmnlint-plugin-camunda-compat that referenced this pull request Jun 12, 2023
@philippfromme
chore: separate C7 and C8 rules
b3371dc 
Requires bpmn-io/bpmnlint#99
@nikku
Copy link
Member

nikku commented Jun 12, 2023

Awesome :).

@nikku
Copy link
Member

nikku commented Jun 12, 2023

Will go ahead and release this.

@nikku
Copy link
Member

nikku commented Jun 12, 2023

Never mind. This worked great here but utterly fails on master. Gotta investigate what is going on there.

@nikku
Copy link
Member

nikku commented Jun 12, 2023

This did the trick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikku nikku Awaiting requested review from nikku

@barmac barmac Awaiting requested review from barmac

@marstamm marstamm Awaiting requested review from marstamm

@smbea smbea Awaiting requested review from smbea

Assignees

@nikku nikku

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@philippfromme @nikku @barmac