-
-
Notifications
You must be signed in to change notification settings - Fork 380
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: Removed unneccessary logs and updated the version for sentry #1050
Conversation
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
Apply Sweep Rules to your PR?
|
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.
Please address the comments
main.go
Outdated
@@ -15,7 +15,7 @@ import ( | |||
// see https://goreleaser.com/customization/build/ | |||
|
|||
var version string | |||
var dsn string | |||
var Dsn string |
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.
why did you change this ..?
main.go
Outdated
@@ -34,12 +34,14 @@ func main() { | |||
version = "2-dev" | |||
} | |||
fmt.Println(logo, " ") | |||
fmt.Printf("version: %v\n\n", version) | |||
fmt.Printf("%v\n\n", version) |
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.
why did you change this..?
utils/utils.go
Outdated
@@ -10,6 +10,8 @@ import ( | |||
sentry "github.com/getsentry/sentry-go" | |||
) | |||
|
|||
var VersionForSentry string |
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.
The naming itself is wrong, are we using this version sentry..? just keep it keployVersion.
Signed-off-by: Pranshu Srivastava <iampranshu24@gmail.com>
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.
Ship it!
Related Issue
Currently a hardcoded version is injected into sentry . Need to inject correct keploy version.
Closes: #1095 #1032
Describe the changes you've made
Updated the location of keploy logs and deleting the logs as soon as keploy stops. Also removed unncessary logs from the utils package, also updated the version updating mechanism
Type of change
Checklist: