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

feat: change codegen to use php 8.1 #100

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

ctran88
Copy link
Contributor

@ctran88 ctran88 commented Jan 6, 2025

What's New?

generate.sh

  • changed to use php-nextgen instead of php to make the generated code use PHP 8.1
  • only copies the source code into generated/lib to "whitelist" instead of removing unnecessary meta files to "blacklist" them
    • this also addresses several new files that were not generated before, but are still not needed now
  • updates generator script to codemod generated API definitions so that the return values are void instead of a union of error models
    • for some reason php-nextgen is generating return types like OpenApi\Client\Model\Model401Error|OpenApi\Client\Model\Model404Error|etc... instead of void like php used to, so this was necessary to correct the type errors from those operations

custom/

  • changes hand-written models to use enums instead of strings

generated/

  • deleted meta or ci files that are not necessary
  • ran generate.sh openapi.json against the current published spec. most of the changes seem to be
    • use Class instead of using the global import syntax \Class
    • adding types to variables, params, and function return signatures
    • adding a type unioned |null where it was previously implicitly typed as such

test with

BRANCH=PSG-5559-use-enums task php:test-release

Screenshots (if appropriate):

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have manually tested my code thoroughly
  • I have added/updated inline documentation for public facing interfaces if relevant
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing integration and unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Additional context

@ctran88 ctran88 force-pushed the PSG-5559-use-enums branch 2 times, most recently from 125eb49 to 0c2763c Compare January 6, 2025 23:24
readonly class MagicLinkArgsBase
{
public function __construct(
public readonly string $type,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed unnecessary readonly access modifiers to allow devs to update the arg objects after instantiation

@@ -51,15 +51,13 @@
"autoload": {
"psr-4": {
"OpenAPI\\Client\\": [
"generated/lib/",
"custom/models/"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this directory and the one below that is also removed no longer exist

@ctran88 ctran88 marked this pull request as ready for review January 6, 2025 23:26
@ctran88 ctran88 requested a review from a team January 6, 2025 23:26
@ctran88 ctran88 force-pushed the PSG-4854-update-min-lang-version branch from 0cdcc4d to f3c039f Compare January 7, 2025 17:35
@ctran88 ctran88 force-pushed the PSG-5559-use-enums branch from 0c2763c to 22f0c1e Compare January 7, 2025 17:36
@ctran88 ctran88 force-pushed the PSG-4854-update-min-lang-version branch from f3c039f to 556c4ae Compare January 7, 2025 21:49
@ctran88 ctran88 force-pushed the PSG-5559-use-enums branch from 22f0c1e to a5a5897 Compare January 7, 2025 21:56
Base automatically changed from PSG-4854-update-min-lang-version to main January 8, 2025 21:59
@ctran88 ctran88 force-pushed the PSG-5559-use-enums branch from a5a5897 to cd78ec1 Compare January 8, 2025 22:19
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.

1 participant