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 flagsets typo in storage #249

Merged
merged 3 commits into from
Sep 25, 2023
Merged

Fix flagsets typo in storage #249

merged 3 commits into from
Sep 25, 2023

Conversation

emmaz90
Copy link
Contributor

@emmaz90 emmaz90 commented Sep 21, 2023

Javascript commons library

What did you accomplish?

  • replace Flagset with FlagSet
  • no-op getNamesByFlagSets method for in redis & pluggable storage

How do we test the changes introduced in this PR?

Extra Notes

@emmaz90 emmaz90 requested a review from a team as a code owner September 21, 2023 17:49
@@ -17,6 +18,7 @@ export abstract class AbstractSplitsCacheAsync implements ISplitsCacheAsync {
abstract getChangeNumber(): Promise<number>
abstract getAll(): Promise<ISplit[]>
abstract getSplitNames(): Promise<string[]>
abstract getNamesByFlagSets(flagsets: string[]): Promise<ISet<string>>
Copy link
Contributor

Choose a reason for hiding this comment

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

Emma, replace also the occurrences of flagsets to flagSets. Include others like flagsetsFilter to flagSetsFilter, etc.

@@ -77,6 +78,11 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync {
}
return false;
}
/** NO-OP */
// eslint-disable-next-line @typescript-eslint/no-unused-vars
getNamesByFlagSets(flagsets: string[]): ISet<string> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this is not abstract?

@emmaz90 emmaz90 merged commit 856f8b7 into sdks-7437 Sep 25, 2023
@emmaz90 emmaz90 deleted the storage-typo branch September 25, 2023 14:52
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