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(pubsub): add support for links in pubsub triggers #1235

Merged
merged 2 commits into from
Dec 14, 2022

Conversation

chris-h-phillips
Copy link
Contributor

It is nice how how Jenkins triggers display a link back to the build that triggered the pipeline. This keys off the link and linkText properties in the Trigger object. I would like to see this same behavior supported for pubsub triggers. This change allows senders of pubsub events to add link and linkText in the trigger message payload, and thus set a link that will be displayed in the pipeline execution UI.

I tested this out on our spinnaker installation and it renders the link like a champ:

image

.atParameters(parameters)
.atPayload(payload)
.atEventId(pubsubEvent.getEventId());
String linkText = (String) payload.get("linkText");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it too paranoid to be worried about payload.get("linkText"); or payload.get("link"); being something besides a string? How about some instanceof checks first?

It is nice how how Jenkins triggers display a link back to the build that triggered the pipeline. This keys off the link and linkText properties in the Trigger object. I would like to see this same behavior supported for pubsub triggers. This change allows senders of pubsub events to add link and linkText in the trigger message payload, and thus set a link that will be displayed in the pipeline execution UI.
@dbyron-sf dbyron-sf added the ready to merge Approved and ready for merge label Dec 14, 2022
@mergify mergify bot added the auto merged label Dec 14, 2022
@mergify mergify bot merged commit 827532f into spinnaker:master Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants