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

update console stub return types and docblocks #493

Merged
merged 3 commits into from
Jan 22, 2024

Conversation

browner12
Copy link
Contributor

  • add void return type
  • remove unnecessary docblocks

syncs with a docs change

- add `void` return type
- remove unnecessary docblocks

syncs with a [docs change](laravel-zero/docs@c402dd2)
Copy link
Member

@owenvoke owenvoke left a comment

Choose a reason for hiding this comment

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

I was thinking of merging this, however I remembered the issues which people brought up when Laravel added return types to these stubs.

Related, I do think we should update the stubs within Laravel Zero to use the {{ class }} format that Laravel uses (rather than DummyClass, etc.)

src/Commands/stubs/console.stub Outdated Show resolved Hide resolved
remove return type

Co-authored-by: Owen Voke <[email protected]>
@browner12
Copy link
Contributor Author

should we also update the "DummyNameSpace"?

@owenvoke
Copy link
Member

Yes, and the signature. 👍🏻 Basically like the core console.stub, but including the additional methods we have.

- use "namespace" and "command" variables rather than "Dummy" string replacement
- update docblock descriptions to match `laravel/framework` changes
@browner12
Copy link
Contributor Author

okay, everything should be synced up now.

I tried this stub on a local Laravel Zero app I have and the file generated correctly.

Should be good to review.

@owenvoke owenvoke merged commit aac9c1b into laravel-zero:master Jan 22, 2024
8 checks passed
@browner12 browner12 deleted the AB-stub-return-types branch January 22, 2024 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants