-
Notifications
You must be signed in to change notification settings - Fork 147
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
Update XscGitInfoContext struct #1089
base: dev
Are you sure you want to change the base?
Conversation
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.
Nice, checkout my comments
type PullRequestContext struct { | ||
PullRequestId int `json:"pull_request_id,omitempty"` | ||
PullRequestTitle string `json:"pull_request_title,omitempty"` | ||
TargetBranchName string `json:"target_branch_name,omitempty"` |
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 would extract this variable from this struct to stand at XscGitInfoContext
as: Target *CommitContext
If we will want more information about the branch it will be a breaking changes at the current state.
requestContent, err := json.Marshal(event) | ||
if err != nil { | ||
return "", errorutils.CheckError(err) | ||
func (vs *AnalyticsEventService) AddGeneralEvent(event XscAnalyticsGeneralEvent, xrayVersion string) (string, error) { |
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.
you can also detect the version instead of getting the version as arg if you want:
xrayVersion, err := xrDetails.GetVersion()
if err != nil {
return errors.New("Couldn't get Xray version. Error: " + err.Error())
}
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 first tried this option but had two thoughts:
- I think there’s no reason to make another API call if we already have this information.
- In cases when a user is using the xsc-service before it became part of Xray, the vs.XrayDetails.GetVersion() will be nil.
Update AddGeneralEvent and XscGitInfoContext structs due to changes done in XSC (as part of PR view epic).

The new XscGitInfoStruct should look like this: