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

Allow info to be null on getProfileInfo #232

Merged
merged 3 commits into from
Sep 18, 2020
Merged

Conversation

Half-Shot
Copy link
Contributor

We use this in various places on the Slack bridge to get the full profile of a user.

@Half-Shot Half-Shot requested a review from a team September 16, 2020 17:38
Copy link
Contributor

@jaller94 jaller94 left a comment

Choose a reason for hiding this comment

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

Looks good to me, however, I'm concerned that the two string may be too specific.

@@ -520,7 +520,7 @@ export class Intent {
* @return A Promise that resolves with the requested user's profile
* information
*/
public async getProfileInfo(userId: string, info: string, useCache=true) {
public async getProfileInfo(userId: string, info: "avatar_url"|"displayname"|null = null, useCache = true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are "avatar_url"|"displayname" the only values that profile.get() supports?
TypeScript becomes a pain if Types are incorrectly too strict.

But if they are the only reasonable values, great to help bridges using this library to spell them correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The spec doesn't allow arbitrary keys, and it's been that way for as long as Matrix has been around. We can update this if the spec changes at some point.

Copy link
Contributor

@jaller94 jaller94 left a comment

Choose a reason for hiding this comment

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

Ah. the builds are failing.

@Half-Shot Half-Shot requested a review from jaller94 September 17, 2020 09:16
Copy link
Contributor

@jaller94 jaller94 left a comment

Choose a reason for hiding this comment

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

giphub

@Half-Shot Half-Shot merged commit e3dec3e into develop Sep 18, 2020
@jaller94 jaller94 deleted the hs/null-get-profile-info branch September 18, 2020 10:56
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.

2 participants