-
Notifications
You must be signed in to change notification settings - Fork 56
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 the Slack Specification and rewrite all the patches #45
Conversation
damienalexandre
commented
Aug 7, 2019
- some patches are removed thanks to Slack updating his specification
- some remains, specifically the one handling:
- message attachments;
- timestamps as string instead of number.
Looks like Slack changed some object definition and used We should probably apply a patch to transform back the user model in one class. Should we also fix other models that are now described with the |
You were right, the Slack specification introduced some polymorphic objects that cannot be handled gracefully in the generated client. I added two patches to merge those objects.
Could you please test again? |
@damienalexandre does this fix #7 ? |
As far as I can guess, yes... maybe? But it should be ok on master too... did you try again recently? Look at this part: slack-php-api/generated/Endpoint/UsersList.php Lines 67 to 81 in fc79868
If the status is not 200, the slack-php-api/generated/Model/UsersListGetResponsedefault.php Lines 13 to 22 in fc79868
|
The PR now works great on https://github.com/jolicode/secret-santa. Great jobs 👏 |