-
Notifications
You must be signed in to change notification settings - Fork 4
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
[Build-trigger-45] Providing more robust messages #52
Conversation
} | ||
logger.infof("Triggering build for product %s by %s", buildInfo, email); | ||
|
||
buildTrigger.triggerBuild(BuildJMSTriggerPayload.from(buildInfo, email)); | ||
|
||
return Response.ok("Triggered build for " + buildInfo.gitRepo + ":" + buildInfo.projectVersion).build(); | ||
return Response.ok("Build triggered successfully for repository " + buildInfo.gitRepo |
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.
Does it even make sense to have these string messages here? I'm not sure if we even use them on FE.
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.
Yes, the response text is currently being used in notification alerts.
return Response.ok(buildInfo).build(); | ||
|
||
logger.infof("Successfully fetched build information for URL: %s", url); | ||
return Response.ok(buildInfo) |
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.
It seems like this is returning only the String instead of JSON with buildInfo and therefore the automatic form fill does not work.
From the getBuildInfo
endpoint, I am getting:
Build information fetched successfully for URL: https://github.com/shrinkwrap/resolver/releases/tag/3.3.1
,
instead of:
{ "tag":"https://github.com/shrinkwrap/resolver/releases/tag/3.3.1", "gitRepo":"https://github.com/shrinkwrap/resolver", "projectVersion":"3.3.1", "commitSha":"138f2f81d186774758409a421c227f3a0ec6031c", "streams":null }
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.
OMG thank you!
31b8845
to
a67fbb6
Compare
LGTM, thanks. |
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.
LGTM
resolves #45