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

Add author_subname type #1621

Merged
merged 1 commit into from
May 17, 2023
Merged

Add author_subname type #1621

merged 1 commit into from
May 17, 2023

Conversation

mztnnrt
Copy link
Contributor

@mztnnrt mztnnrt commented May 5, 2023

Hello! I would like to start by expressing my sincere appreciation for your consistent contributions to this project.
Thank you so much!

Summary

Add author_subname for message, the section indicated by the arrow.

スクリーンショット 2023-05-05 19 03 22


Other language API Client ex. slack-go/slack

https://github.com/slack-go/slack/blob/c4095cb17cf21c2b2b9c459d5512d14ca5143f64/attachments.go#L70-L71

AuthorName    string `json:"author_name,omitempty"`
AuthorSubname string `json:"author_subname,omitempty"`

Requirements (place an x in each [ ])

@salesforce-cla
Copy link

salesforce-cla bot commented May 5, 2023

Thanks for the contribution! Before we can merge this, we need @mztnnrt to sign the Salesforce Inc. Contributor License Agreement.

@mztnnrt mztnnrt changed the title chore: add author_subname? Add author_subname for Message API May 5, 2023
@mztnnrt mztnnrt marked this pull request as ready for review May 5, 2023 11:47
@mwbrooks mwbrooks added area:typescript issues that specifically impact using the package from typescript projects pkg:types applies to `@slack/types` semver:patch labels May 6, 2023
@mwbrooks mwbrooks added this to the [email protected] milestone May 6, 2023
@mwbrooks mwbrooks changed the title Add author_subname for Message API Add author_subname type May 6, 2023
@mwbrooks
Copy link
Member

mwbrooks commented May 6, 2023

Hey @mztnnrt 👋🏻

Thanks a bunch for the pull request 🙇🏻 It's wonderful to hear that you've finding the project helpful and we always appreciate pull requests that continue to improve the project for everyone!

I'll be honest, I didn't know the author_subname property existed! Digging a little deeper, it appears to be a legacy property for message attachments. It's not documented on api.slack.com, which is probably why we have not added the type definition to the SDK.

Regardless, for those in-the-know, the property is still supported by message attachments and displays correctly in Slack. It's quite slick!
image

How do the other maintainer's feel about adding this type? It seems good to me.

@mztnnrt
Copy link
Contributor Author

mztnnrt commented May 8, 2023

Hi @mwbrooks, Thanks your reply ! 🙏

It's not documented on api.slack.com,

Yes ! In fact, I also don't know how long Slack will support author_subname, but now I think it useful for display_id and so on.
Therefore, i also feel good to add it. I think it is good to consider it positively.

@seratch
Copy link
Member

seratch commented May 8, 2023

@mwbrooks

How do the other maintainer's feel about adding this type? It seems good to me.

I also think this is a good addition 👍 We may want to add the field here too, but missing it there at this moment doesn't necessarily prevent us from adding the actually working thing to the SDKs.

I can work on Python/Java SDKs later on.

@seratch
Copy link
Member

seratch commented May 9, 2023

My updates on this:

@mztnnrt
Copy link
Contributor Author

mztnnrt commented May 13, 2023

Hi @seratch 👋 Thanks for your adjustment about this subname!
Please continue your support 🙏

P.S. I always read your article qiita.com about Slack and it's very useful, thanks.

@seratch seratch merged commit 708ee5e into slackapi:main May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:typescript issues that specifically impact using the package from typescript projects cla:signed pkg:types applies to `@slack/types` semver:patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants