Skip to content

Comments

Isovector/brig servant 2022#2556

Merged
mdimjasevic merged 4 commits intodevelopfrom
isovector/brig-servant-2022
Jul 18, 2022
Merged

Isovector/brig servant 2022#2556
mdimjasevic merged 4 commits intodevelopfrom
isovector/brig-servant-2022

Conversation

@battermann
Copy link
Contributor

@battermann battermann commented Jul 13, 2022

Part of SQCORE-1166. Ports the Brig UserHandle API to use Servant.

Leaving the last two checkboxes unchecked because I don't know the answer to them.

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@battermann battermann temporarily deployed to cachix July 13, 2022 07:48 Inactive
@battermann battermann temporarily deployed to cachix July 13, 2022 07:54 Inactive
@mdimjasevic
Copy link
Contributor

@isovector , we've been experiencing issues with the CI, which then prevents this PR (and all the other PRs) from getting a green light. We'll retrigger the build in CI as soon as the issues are resolved.

@battermann battermann requested a review from mdimjasevic July 14, 2022 14:00
@isovector
Copy link
Contributor

CI is green --- someone mind reviewing it? @elland @battermann @mdimjasevic thanks!

@mdimjasevic
Copy link
Contributor

CI is green --- someone mind reviewing it? @elland @battermann @mdimjasevic thanks!

I see that two endpoints listed in https://wearezeta.atlassian.net/browse/SQSERVICES-1642 are not covered by this PR:

  • GET /users/:uid/rich-info
  • GET /teams/:tid/search

Should they be part of the PR or should they come in a follow-up PR?

@isovector
Copy link
Contributor

isovector commented Jul 15, 2022

@mdimjasevic I'm happy to merge this as is and follow up in a second PR.

Copy link
Contributor

@mdimjasevic mdimjasevic 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!

@mdimjasevic mdimjasevic merged commit c4d1e24 into develop Jul 18, 2022
@mdimjasevic mdimjasevic deleted the isovector/brig-servant-2022 branch July 18, 2022 11:22
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