Skip to content

Conversation

pcapriotti
Copy link
Contributor

@pcapriotti pcapriotti commented Feb 3, 2022

This PR is the brig counterpart to #2022, i.e. it replaces uses of servant-generic in brig with Named routes, and splits the API tree into sections.

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.

@pcapriotti pcapriotti requested a review from supersven February 3, 2022 15:23
Copy link
Contributor

@supersven supersven left a comment

Choose a reason for hiding this comment

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

The translation to the new format looks good. IMHO it's better to read. 👍

Only a few minor questions...

@pcapriotti pcapriotti force-pushed the pcapriotti/brig-named-routes branch from 7a4d50c to 8e6d28c Compare February 4, 2022 06:51
@pcapriotti
Copy link
Contributor Author

pcapriotti commented Feb 4, 2022

Thanks for reviewing. Unfortunately, since the PR contained a merge of #2091 (to avoid having to deal with conflicts later), you have also looked at some changes from the other PR. I have rebased now.

Fixed the typo and readded the missing Summarys (thanks for spotting that!). Would you mind having another quick look?

@pcapriotti pcapriotti requested a review from supersven February 4, 2022 06:53
Copy link
Contributor

@supersven supersven left a comment

Choose a reason for hiding this comment

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

Looks great 👍

@pcapriotti pcapriotti merged commit 09710ed into develop Feb 4, 2022
@pcapriotti pcapriotti deleted the pcapriotti/brig-named-routes branch February 4, 2022 08:21
@fisx fisx mentioned this pull request Feb 18, 2022
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