search

jolicode / slack-php-api

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

Add locale to Conversation #116

Closed ghost closed 3 years ago

ghost commented 3 years ago

Hi, i just want to add locale to Conversation.

I through it will be just fast improvement, but the test failed. If i made something wrong could you add it yourself?

pyrech commented 3 years ago

Hi @ASR-WIR

Thanks for your contribution. The files you edited are not meant to be updated manually. In fact the whole SDK is generated automatically from the official API spec provided by Slack (and locally override in our repository). We have some documentation explaining how to update and override the spec.

So in your case, you need to edit the patched JSON spec to add the missing property locale property to objs_conversation modele, then regenerate the SDK (steps are better described in the documentation). Do not hesitate if If you need help or want us to add the missing prop.

PS: concerning the failing tests, yes, looks like I recently messed the CI for PR opened from forks, I will fix that ASAP

ghost commented 3 years ago

Thank you for fast response and explanation! It is great to know that it is possible to generate SDK from swagger api! Thank you!

I tried to add it (pic) but after generating command the locale field does not appear in generated code. Could you add missing locale to conversation object?

Should i write to Slack team for fix?

image

pyrech commented 3 years ago

Slack do not update their specs quite often but yes you could open an issue on their repository : https://github.com/slackapi/slack-api-specs

In the meantime, we will try to add the locale property in our SDK

pyrech commented 3 years ago

@ASR-WIR For your information, I just merged #121 which add the missing locale property on Conversation object. It should be released in a few days on new minor version.