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

Add feature flag "conferenceCalling" #1683

Merged
merged 17 commits into from
Jul 29, 2021
Merged

Conversation

smatting
Copy link
Contributor

@smatting smatting commented Jul 27, 2021

https://wearezeta.atlassian.net/browse/SQSERVICES-681

This PR also:

  • refactor some of the feature endpoint implementation
  • adds to test that all simple flags should emit a feature-config.update event

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.
  • If end-points have been added or changed: the endpoint / config-flag checklist (see Wire-employee only backend wiki page) has been followed.
  • If a schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
  • Section Unreleased of CHANGELOG.md contains the following bits of information:
    • A line with the title and number of the PR in one or more suitable sub-sections.
    • If /a: measures to be taken by instance operators.
    • If /a: list of cassandra migrations.
    • If public end-points have been changed or added: does nginz need upgrade?
    • If internal end-points have been added or changed: which services have to be deployed in a specific order?

@smatting smatting changed the title Add feature flag "conferenceCalling" [skip ci] Add feature flag "conferenceCalling" Jul 27, 2021
Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

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

Could you update the docs/config-options.md and explain what it means for this to be enabled or disabled, and what the default is?

@smatting smatting marked this pull request as ready for review July 28, 2021 12:36
@smatting
Copy link
Contributor Author

Could you update the docs/config-options.md and explain what it means for this to be enabled or disabled, and what the default is?

Done

@smatting smatting requested review from jschaul and fisx July 28, 2021 12:37
Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

LGTM!

No point in changing this now, but I would have preferred 2-3 smaller PRs (at least the unrelated tests should have been trivial to do separately).

@@ -17,8 +17,6 @@ license: AGPL-3
build-type: Simple
extra-source-files:
test/resources/integration-ca.pem
test/resources/integration-leaf-key.pem
test/resources/integration-leaf.pem
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems unrelated?

@smatting smatting merged commit 31479d9 into develop Jul 29, 2021
@smatting smatting deleted the SQSERVICES-681-conferenceCalling branch July 29, 2021 12:43
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