Skip to content

Conversation

turt2live
Copy link
Member

See matrix-org/synapse#4303

This was done with a community hat on:
Signed-off-by: Travis Ralston travis@t2bot.io

@turt2live turt2live requested a review from a team December 18, 2018 03:09
@@ -41,6 +42,42 @@ sub matrix_add_room_account_data
);
}

=head2 matrix_get_account_data

matrix_get_account_data( $user, $type, $content )->get;
Copy link
Member

Choose a reason for hiding this comment

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

why does this have a ->get?

Copy link
Member Author

Choose a reason for hiding this comment

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

copy/paste. I guess it's not needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

looks like the other functions here also have it...

Copy link
Member

Choose a reason for hiding this comment

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

Github lost my comment that I noticed that :(

I have no idea what its doing with its life

@richvdh
Copy link
Member

richvdh commented Jan 2, 2019

I guess this needs to wait for matrix-org/synapse#4303 to land?

@turt2live
Copy link
Member Author

yes, and the status of the synapse PR is waiting for the MSC bot to run its course, which it did over the holidays.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

some negative-case tests (eg: what happens if you try to get someone else's account data, or account data that doesn't exist) would be nice, but this is good anyway

@richvdh richvdh merged commit 57baf45 into develop Jan 7, 2019
@turt2live turt2live deleted the travis/get-account-data branch January 7, 2019 14:40
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.

3 participants