Skip to content

Add identity server test#594

Merged
erikjohnston merged 5 commits intodevelopfrom
erikj/id_server_tests
Apr 4, 2019
Merged

Add identity server test#594
erikjohnston merged 5 commits intodevelopfrom
erikj/id_server_tests

Conversation

@erikjohnston
Copy link
Member

No description provided.

@erikjohnston erikjohnston requested a review from a team April 1, 2019 14:29
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.

again a few nitpicks

@@ -0,0 +1,31 @@
test "Can bind 3PID via home server",
Copy link
Member

Choose a reason for hiding this comment

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

probably ought to have a copyright comment.

Also: is it worth starting a new subdir?

Copy link
Member Author

Choose a reason for hiding this comment

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

probably ought to have a copyright comment.

Almost nowhere else has copyright notices, which may or may not be problematic.

Also: is it worth starting a new subdir?

I don't think so tbh, we can always do it down the line?

Copy link
Member

Choose a reason for hiding this comment

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

Almost nowhere else has copyright notices, which may or may not be problematic.

well, they should do, and we should get into the habit of adding them on new files. Don't mind leaving it for now though

Also: is it worth starting a new subdir?

I don't think so tbh, we can always do it down the line?

k then

@erikjohnston erikjohnston requested a review from richvdh April 3, 2019 10:20
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.

lgtm

@erikjohnston erikjohnston merged commit 9af8e28 into develop Apr 4, 2019
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