Skip to content

Remove public signal check from signal prefix #732

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

Merged
merged 3 commits into from
Jan 27, 2021
Merged

Remove public signal check from signal prefix #732

merged 3 commits into from
Jan 27, 2021

Conversation

chinandrew
Copy link
Contributor

@chinandrew chinandrew commented Jan 26, 2021

Description

Removes footgun for wip signals by having the add_prefix signal not auto-add "wip_" to non-public signals.

grep -r '"wip_signal":' * shows everything is "wip_signal": "", or "wip_signal": [""], in the repo, so it's hard to say if any signals would be affected by this change. I had to update some cdc covidnet changes, not sure if those are actually wip and we need to add it to the params json

Changelog

Itemize code/test/documentation changes and files added/removed.

  • Remove public_signal() function
  • Have add_prefix just return the input signals if wip_signals is false or ''
  • Fix cdc covidnet tests.

Fixes

  • The OKRs "Remove footgun in WIP signal rename widget"

@chinandrew chinandrew requested a review from krivard January 26, 2021 22:48
Copy link
Contributor

@krivard krivard left a comment

Choose a reason for hiding this comment

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

lgtm

covidnet is not scheduled in production, so we won't get any accidental releases there.

@krivard krivard merged commit 3b4360c into main Jan 27, 2021
@krivard krivard deleted the remove-wip branch January 27, 2021 14:11
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