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

[9.x] Add invokable option to make rule command #42742

Merged
merged 5 commits into from
Jun 21, 2022

Conversation

snarcraft
Copy link
Contributor

Following the PR #42689, this new PR adds an option to make an invokable rule to the existing make rule command.

This minor feature comes with no breaking change, the previous command will still work as usual.
For the end users, this additional option can be useful to generate an invokable rule.

@taylorotwell
Copy link
Member

@timacdonald should we transition to making "invokable" rules the default class based custom rules we document as well as the default type of rules we generate from the CLI?

@timacdonald
Copy link
Member

timacdonald commented Jun 9, 2022

@taylorotwell I do feel that making these the "default" is the way to go. It creates consistency across the rule APIs (Invokable class / Closure based), they have better DX for more complicated rules (as shown in the PR), and is one less decision for developers to make when creating rules e.g. "which rule type do we use".

@taylorotwell taylorotwell added bug and removed bug labels Jun 10, 2022
@taylorotwell
Copy link
Member

Should we be handling the combination of --invokable and --implicit?

@timacdonald
Copy link
Member

Yea, for 9.x I think we should handle both of these. @snarcraft, if you have the space, could you add support for the following....

artisan make:rule --invokable
artisan make:rule --invokable --implicit

If you don't have space right now, just let me know and I can jump in for ya.

Will convert this one to a draft until we have it ready.

@timacdonald timacdonald marked this pull request as draft June 13, 2022 23:45
@snarcraft
Copy link
Contributor Author

@timacdonald there you go!

@snarcraft snarcraft marked this pull request as ready for review June 14, 2022 18:19
…t.stub

Co-authored-by: Tim MacDonald <hello@timacdonald.me>
@taylorotwell taylorotwell merged commit 8190a1d into laravel:9.x Jun 21, 2022
@snarcraft snarcraft deleted the invokable-rule-command branch March 27, 2024 20:23
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.

3 participants