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

Fix for the #1865 #1889

Merged
merged 4 commits into from Feb 28, 2019
Merged

Fix for the #1865 #1889

merged 4 commits into from Feb 28, 2019

Conversation

ma1uta
Copy link

@ma1uta ma1uta commented Feb 15, 2019

Add the m.push_rules schema.

It needs to check carefully the schema.

Signed-off-by: Anatoly Sablin sablintolya@gmail.com

@turt2live turt2live requested review from turt2live and a team and removed request for turt2live February 16, 2019 05:34
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.

looks plausible to me, but it's a shame we're directly duplicating the definitions from api/client-server/definitions/push_ruleset.yaml etc. @turt2live: do you know if there is a way these could be shared to save duplication?

@turt2live
Copy link
Member

Using $ref at every available opportunity is the best deduplication approach. PDUs in the s2s spec are a great example of this.

@turt2live turt2live removed their assignment Feb 21, 2019
@richvdh
Copy link
Member

richvdh commented Feb 21, 2019

Ah yes. there are plenty of examples of APIs which refer to event schemas.

@ma1uta: could you rework this to remove the duplication?

@ma1uta
Copy link
Author

ma1uta commented Feb 24, 2019

@richvdh done.

@turt2live
Copy link
Member

A changelog (clarification) and this is ready to merge, I guess :)

@ma1uta
Copy link
Author

ma1uta commented Feb 28, 2019

@richvdh @turt2live @KitsuneRal done. I hope this is enough to merge :)

@turt2live
Copy link
Member

Lots of green checkmarks compels me to merge this. Thanks for the PR!

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.

None yet

4 participants