Road to 1.0

[sagikazarmark]

Preflight Checklist

• I have searched the issue tracker for an issue that matches the one I want to file, without success.

Problem Description

The library has been around for some time and with some recent improvements, it might be ready for 1.0

Proposed Solution

Review the project and check what needs to be deprecated, upgraded, brought in line with latest practices.

(eg. after dropping PHP <8.1 support, we might want to upgrade the generated code and the shared library, etc)

Alternatives Considered

No response

Additional Information

@antonkomarev @SpencerMalone any thoughts?

[SpencerMalone]

Unless we want to go towards like 8.4+ and drop 8.1-8.3 support (which I don't think we should for 1.0 release), I don't think there's dramatic syntax changes we need to make.

My personal checklist would be:

• Some of the interfaces we've been talking about (already in progress)
• I think the async client functions would be really nice (already in progress)
• A lot of the generated code is still docblock driven typing (ex: https://github.com/twirphp/twirp/blob/master/protoc-gen-twirp_php/templates/service/Server.php.tmpl#L39). I think this should be resolved and generally would be relatively easy. My personal policy is anything not an array should be strongly typed if possible. (I'd be happy to open some PRs on this)
• There's a few function params that are missing any docblock, such as https://github.com/twirphp/twirp/blob/master/protoc-gen-twirp_php/templates/service/Server.php.tmpl#L178. Even an array<mixed> would be good IMO.
• Upgrade phpstan to the latest and set that as a dev dependency maybe? Along with there, I personally think there is a minor TODO of running generation on the example and ensuring that example/ is part of the phpstan CI run. I think doing that will shake out a lot of the things from point 3+4, and would have caught stuff like Fix AbstractClient.php.tmpl property type #219.

[antonkomarev]

I agree with @SpencerMalone

And if we doesn't introduce any backward incompatible changes, we just may increase minor version. New interface is backward compatible, and it's too small for the major release.

[sagikazarmark]

@SpencerMalone sounds good!

@antonkomarev a first major release would signal maturity and confidence we won't be breaking any interfaces.

[SpencerMalone]

@sagikazarmark do you prefer lots of small PRs with minor fixes, or a mega PR that lumps multiple fixes together?

[sagikazarmark]

Depends on the fixes. I'm okay with lots of small commits if they are reviewable. Lots of large, independent commits should probably be broken up into smaller PRs, except when it would make merging harder (eg. due to merge conflicts).

I'm afraid to call this common sense, because it got me into trouble on more than one occasions. :D Do what makes sense to you.

[SpencerMalone]

I'm feeling pretty good about things, how are y'all feeling? Should we do a RC and then wait a bit before announcing it as 1.0.0?

[sagikazarmark]

Thanks for all your work!

Yeah, I think we are ready.

Either an RC or one last minor release. WDYT?

RC might send a good message though.

hm...

Yeah, RC sounds good.

[SpencerMalone]

RC is my instinct as well!

[sagikazarmark]

It's out: https://github.com/twirphp/twirp/releases/tag/v1.0.0-rc.2

(messed up RC1 GHA)