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

feat: conditionally render LSP template [IDE-233] #457

Merged
merged 3 commits into from
May 22, 2024

Conversation

cat2608
Copy link
Contributor

@cat2608 cat2608 commented May 14, 2024

Description

This PR uses the feature flag status to render Issue details provided by Language Server.

Note: styling and template updates will be addressed in next PRs

Checklist

  • Tests added and all succeed
  • Linted
  • CHANGELOG.md updated
  • README.md updated, if user-facing

Screenshots / GIFs

FF Disabled FF Enabled
before after

@cat2608 cat2608 requested a review from a team as a code owner May 14, 2024 15:57
@@ -43,6 +44,7 @@ export class CodeSettings implements ICodeSettings {
}
await this.contextService.setContext(SNYK_CONTEXT.CODE_ENABLED, codeEnabled);
await this.contextService.setContext(SNYK_CONTEXT.CODE_LOCAL_ENGINE_ENABLED, localCodeEngineEnabled);
this.config.setFeatureFlag(FEATURE_FLAGS.consistentIgnores, codeEnabled);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this is the right place to put it, ideally we would have a more global way of updating feature flag status even if this is for Snyk Code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, this is not the correct implementation.
Working on it 💪

Copy link
Contributor

@teodora-sandu teodora-sandu left a comment

Choose a reason for hiding this comment

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

Even though I'm not sure about the way we manage the feature flags in VSCode, I'm not going to block this PR since it's unrelated and this is an improvement on what we currently have. Thanks for doing this!

cat2608 added 3 commits May 22, 2024 11:30
- Update `CodeSettings` to set the `consistentIgnores` feature flag based LSP command.
- Modify unit tests to mock `setFeatureFlag` method in `IConfiguration`.
@cat2608 cat2608 force-pushed the feat/IDE-233-render-lsp-template branch from e84831b to 1994154 Compare May 22, 2024 10:34
@cat2608 cat2608 merged commit 02b67c8 into main May 22, 2024
7 checks passed
@cat2608 cat2608 deleted the feat/IDE-233-render-lsp-template branch May 22, 2024 10:50
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