Skip to content

Conversation

@paulbalandan
Copy link
Member

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide
@paulbalandan paulbalandan force-pushed the phpstan-extension-for-factories branch 3 times, most recently from 8627354 to 3a20591 Compare June 14, 2023 07:20
@paulbalandan paulbalandan force-pushed the phpstan-extension-for-factories branch from 3a20591 to 7a12444 Compare June 30, 2023 07:16
@kenjis
Copy link
Member

kenjis commented Jul 2, 2023

Why do you need this?
Of course the current PHPDoc types for them are not perfect, but
I think all return types for them in this repository are now correct without this PR.

If we need this, it should be another package?
Because we can't use this feature with app starter easily.

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

@paulbalandan I agree with kenjis that this might fit better as a standalone package, or even better as part of DevKit where we already collect a number of these scaffolding tools.
I'm excited to try it out though! As far as you know is it working as intended?

@paulbalandan
Copy link
Member Author

@kenjis @MGatner my initial intention of putting it here is to try it out here to catch any bugs (which it did in 4e18871). I can pull this up to a standalone package (even better as a phpstan extension package). Just wanted to show it here first to get some reactions on the behavior.

@MGatner
Copy link
Member

MGatner commented Jul 3, 2023

@paulbalandan that makes sense! I guess there is no real way to demonstrate it publicly otherwise. Yes, please proceed - I'm excited for this enhancement!

@kenjis
Copy link
Member

kenjis commented Jul 3, 2023

Yes, without this we can't detect bugs like that!

@paulbalandan
Copy link
Member Author

Okay. I'll start the phpstan extension this week. Would codeigniter/phpstan-codeigniter4 be okay as package name?

@kenjis
Copy link
Member

kenjis commented Jul 4, 2023

It is only for CI4, so the vendor name should be codeigniter4?

By the way, when we release v5, will the vendor name be codeigniter4?

@paulbalandan
Copy link
Member Author

In coding-standard it was put in the codeigniter vendor name because it's intended to be used in v4 and up (to minimize need of setting up codeigniter5 org etc.), so I think this should also be the case here.

@MGatner
Copy link
Member

MGatner commented Jul 4, 2023

I have advocated to @lonnieezell that with version 5 we need to take over the generic "CodeIgniter" brand and v3 can start adding the number or finally retire.

@paulbalandan
Copy link
Member Author

Closing this for now.

I'll open codeigniter/phpstan-codeigniter package soon.

@paulbalandan paulbalandan deleted the phpstan-extension-for-factories branch July 4, 2023 11:28
@kenjis
Copy link
Member

kenjis commented Jul 4, 2023

Since v3 is already almost unmaintained, it seems like a good idea to make codeigniter/framework v5.

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

Labels

None yet

3 participants