-
-
Notifications
You must be signed in to change notification settings - Fork 658
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
feat: disabled feature dependency #6731
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Code Health Quality Gates: OK
- Declining Code Health: 4 findings(s) 🚩
- Improving Code Health: 0 findings(s) ✅
- Affected Hotspots: 0 files(s) 🔥
Recommended Review Level: Detailed -- Increased risk for defects: The risk is higher as much of the code in this repo (99% of all commits) is written by other authors.
View detailed results in CodeScene
🚩 Declining Code Health (highest to lowest):
- Complex Method DependencyRow.tsx: DependencyRow:FC<{feature:IFeatureToggle}>
- Excess Number of Function Arguments AddDependencyDialogue.tsx: useManageDependency
- Complex Method AddDependencyDialogue.tsx: AddDependencyDialogue
- Complex Method AddDependencyDialogue.tsx: useManageDependency
}; | ||
|
||
export const AddDependencyDialogue = ({ | ||
project, | ||
featureId, | ||
parentFeatureId, | ||
parentFeatureValue, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ New issue: Complex Method
AddDependencyDialogue has a cyclomatic complexity of 10, threshold = 10
Why does this problem occur?
This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring. Read more.
To ignore this warning click here.
const useManageDependency = ( | ||
project: string, | ||
featureId: string, | ||
parent: string, | ||
parentValue: ParentValue, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ New issue: Complex Method
useManageDependency has a cyclomatic complexity of 10, threshold = 10
Why does this problem occur?
This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring. Read more.
To ignore this warning click here.
const useManageDependency = ( | ||
project: string, | ||
featureId: string, | ||
parent: string, | ||
parentValue: ParentValue, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ New issue: Excess Number of Function Arguments
useManageDependency has 5 arguments, threshold = 4
Why does this problem occur?
This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments. Read more.
To ignore this warning click here.
<ConditionallyRender | ||
condition={ | ||
hasParentDependency && !feature.dependencies[0]?.enabled | ||
} | ||
show={ | ||
<FlexRow> | ||
<StyledDetail> | ||
<StyledLabel>Dependency value:</StyledLabel> | ||
<span> | ||
{feature.dependencies[0]?.enabled | ||
? 'enabled' | ||
: 'disabled'} | ||
</span> | ||
</StyledDetail> | ||
</FlexRow> | ||
} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ New issue: Complex Method
DependencyRow:FC<{feature:IFeatureToggle}> has a cyclomatic complexity of 13, threshold = 10
Why does this problem occur?
This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring. Read more.
To ignore this warning click here.
<ConditionallyRender | ||
condition={showDependencyDialogue} | ||
show={ | ||
<LazyOptions | ||
project={project} | ||
featureId={featureId} | ||
parent={parent} | ||
onSelect={setParent} | ||
onSelect={(status) => { | ||
setParentValue({ status: 'enabled' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it should not reset the bottom configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion here so let's gather feedback. I started w/o reset and it felt weird but it's just one opinion. For now it's hidden behind a flag anyway.
About the changes
Feature dependency on a disabled feature (e.g. killswitch)
Important files
Screen.Recording.2024-03-28.at.13.47.46.mov
Details:
Discussion points