-
Notifications
You must be signed in to change notification settings - Fork 240
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
Feature - Update aws-toolkit-commons package #4051
Feature - Update aws-toolkit-commons package #4051
Conversation
- update AmazonQ Code Transform telemetry definitions for new package update
- update AmazonQ Code Transform telemetry definitions for new package update
…thub.com:damntrecky/aws-toolkit-jetbrains into nardeck/amazonq-code-transform-telemetry-updates
@@ -0,0 +1,4 @@ | |||
{ | |||
"type" : "bugfix", | |||
"description" : "Fix telemetry logging for new Amazon Q Code Transform telemetry updates" |
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.
non-userfacing so does not need a changelog
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.
Thanks for letting me know. Should I delete?
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.
happy to keep, doesn't hurt
SonarCloud Quality Gate failed. 0 Bugs No Coverage information Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
@@ -136,7 +136,7 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo | |||
false, | |||
message("codemodernizer.notification.warn.invalid_project.description.reason.missing_content_roots"), | |||
InvalidTelemetryReason( | |||
CodeTransformPreValidationError.EmptyProject | |||
CodeTransformPreValidationError.NoPom |
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.
Is it possible to keep the reason more generic, such as NoBuildFile? Q Transform will support other build tools like Gradle and a generic reason "missing_content_roots" is applicable to these other build tools as well.
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.
Right now its not clear what we are going to support. And right now specifics around maven/gradle are useful metrics to have to determine how many projects we have or do not have with our specific requirements.
@@ -160,7 +160,7 @@ class CodeModernizerManager(private val project: Project) : PersistentStateCompo | |||
false, | |||
message("codemodernizer.notification.warn.invalid_project.description.reason.no_valid_files", supportedBuildFileNames.joinToString()), | |||
InvalidTelemetryReason( | |||
CodeTransformPreValidationError.ProjectSelectedIsNotJava8OrJava11, | |||
CodeTransformPreValidationError.NonMavenProject, |
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.
Once we started supporting other build tools like Gradle, this telemetry would likely become something more generic like NoSupportedProjectType
if project is neither Maven or Gradle. Would it be better to start using that?
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.
Right now specifics around maven/gradle are useful metrics to have to determine how many projects we have or do not have with our specific requirements.
Types of changes
Description
We update the AWS Toolkit Common package and need to update the VSCode package per this PR -> aws/aws-toolkit-common#648
Checklist
License
I confirm that my contribution is made under the terms of the Apache 2.0 license.