jolicode / slack-php-api

:hash: PHP Slack Client based on the official OpenAPI specification
https://jolicode.github.io/slack-php-api/
MIT License
223 stars 56 forks source link

Update SDK to latest Slack version #145

Closed matthewnessworthy closed 2 years ago

matthewnessworthy commented 2 years ago
rm -Rf generated/*
vendor/bin/jane-openapi generate --config-file=.jane-openapi.php
./bin/slack-api-client-generator spec:generate-patch
./bin/slack-api-client-generator spec:update
matthewnessworthy commented 2 years ago

@damienalexandre / @mpyw seems there is an incompatibility with jane-php, please advise

Run rm -rf generated && vendor/bin/jane-openapi generate --config-file=.jane-openapi.php

PHP Fatal error:  Uncaught ArgumentCountError: Too few arguments to function Jane\Component\JsonSchema\Generator\RuntimeGenerator::__construct(), 2 passed in /home/runner/work/slack-php-api/slack-php-api/vendor/jane-php/open-api-2/JaneOpenApi.php on line 44 and exactly 3 expected in /home/runner/work/slack-php-api/slack-php-api/vendor/jane-php/json-schema/Generator/RuntimeGenerator.php:27
matthewnessworthy commented 2 years ago

@damienalexandre / @mpyw issue resolved after updating jane-php/open-api-2 to ^7.2

damienalexandre commented 2 years ago

Thanks a lot @matthewnessworthy for your contributions :tada:

Tests are green on that last commit :clap: :green_heart: ; I'm just wondering about the Jane error, @Korbeil do you have an idea what was going on? (see https://github.com/jolicode/slack-php-api/runs/5709079316?check_suite_focus=true). Thanks :+1:

matthewnessworthy commented 2 years ago

@damienalexandre it was just "jane-php/open-api-2": "~7.1.5", using 7.1.x which did not have the validation changes introduced in jane-php 7.2, causing a function signature mismatch

damienalexandre commented 2 years ago

I don't see the new specification in your PR.

https://github.com/jolicode/slack-php-api/blob/main/src/Command/UpdateSpecificationCommand.php#L46

Did you forgot to commit the resources/slack-openapi.json file?

damienalexandre commented 2 years ago

In fact the official spec didn't move since october 2020: https://github.com/slackapi/slack-api-specs/blob/master/web-api/slack_web_openapi_v2.json

So you are not updating the spec, but adding a new manual overwrite, right?

matthewnessworthy commented 2 years ago

I ran the following

rm -Rf generated/*
vendor/bin/jane-openapi generate --config-file=.jane-openapi.php
./bin/slack-api-client-generator spec:generate-patch
./bin/slack-api-client-generator spec:update

It did not modify resources/slack-openapi.json

This PR fixes the failing workflow step (jane-php/open-api-2 version mismatch) and pulls in any Slack changes It's an attempt to unblock the way for my other 2 pull requests #146 and #147

damienalexandre commented 2 years ago

Running the same commands as you I don't have any specification update (and that's the expected behavior as Slack didn't update their OpenAPI).

I think you mixed some stuffs.

Here is what should be done to fix the SDK generation:

+++ w/composer.json
@@ -34,7 +34,7 @@
         "php-http/multipart-stream-builder": "^1.1"
     },
     "require-dev": {
-        "jane-php/open-api-2": "~7.1.5",
+        "jane-php/open-api-2": "~7.1",
         "symfony/http-client": "^5.4 || ^6.0",
         "nyholm/psr7": "^1.2",
         "friendsofphp/php-cs-fixer": "^3.2.2",
composer update
rm -Rf generated/*
vendor/bin/jane-openapi generate --config-file=.jane-openapi.php

That's all.

Could you rebuild this Pull Request with this change only so we can merge and then look at the other changes?

Thanks a lot!

matthewnessworthy commented 2 years ago

@damienalexandre no problem, done

damienalexandre commented 2 years ago

Perfect, thanks a lot for this fix :+1: