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

Feature: Plugin lists #248

Closed
skjdbg opened this issue May 26, 2023 · 8 comments
Closed

Feature: Plugin lists #248

skjdbg opened this issue May 26, 2023 · 8 comments

Comments

@skjdbg
Copy link
Contributor

skjdbg commented May 26, 2023

While playing around with multiple plugin rules, I encountered 2 issues which I believe could solved through the same feature:

  • The command becomes very long.
  • I need one project for each separate plugin.

Allowing a plugin to return multiple rules would solve the issue since I could compile all my rules in one project and have only one argument to give to svlint.
I tried locally (on Windows) to make it so get_plugin returns a Vec<*mut dyn Rule> instead of a *mut dyn Rule and modifying the load method from Linter to expect that. This solution works great for me, however there might be a better way.

@DaveMcEwan
Copy link
Contributor

IMO, it might be ideal timing for this change. This change would break backwards compatibility with existing plugins, and so would the PR #247. If there has to be a break, it would be nice to break just once (in 2 ways) rather than twice.

@DaveMcEwan
Copy link
Contributor

@skjdbg Can you point to a branch with your demo code? I guess it would really be one branch on svlint and a matching branch on svlint-plugin-sample.

@skjdbg
Copy link
Contributor Author

skjdbg commented Jun 5, 2023

Would https://github.com/skjdbg/svlint-plugin-sample and https://github.com/skjdbg/svlint work ?

I'm not sure if you want me to do a pull request for them or if you want something else.

@DaveMcEwan
Copy link
Contributor

Yes, those would be fine but I can only see one branch (master) on each of those.
I guess the changes are all in 1 commit on each master?

Looks pretty nice to me :)

@DaveMcEwan
Copy link
Contributor

DaveMcEwan commented Jun 5, 2023

In future, it's easier to put changes on a dedicated branch, but these are quite small and only one commit is easy to read.
If you open a pair of pull requests, then GitHub will attribute the changes to you (see also #250).
It's worth noting that these changes conflict with #247, so it's up to dalance which goes in first - the second will need to update their changes to match.

@skjdbg
Copy link
Contributor Author

skjdbg commented Jun 5, 2023

I guess the changes are all in 1 commit on each master?

Yes, to be honest I have never participated in any open-source project so I don't know how I should do things.

@DaveMcEwan
Copy link
Contributor

DaveMcEwan commented Jun 5, 2023

No worries - It looks good to me :)
Would you be happy to be named as a contributor and certify the changes with the Developer Certificate of Origin, for #250? (basically promising that you didn't copy the changes from a copyright-protected source) EDIT, I see you've agreed already, thanks :)

@dalance dalance mentioned this issue Jun 9, 2023
4 tasks
@skjdbg
Copy link
Contributor Author

skjdbg commented Jun 26, 2023

Closing this issue as the feature was added by merging #255.

@skjdbg skjdbg closed this as completed Jun 26, 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

No branches or pull requests

2 participants