bitrise-webhooks icon indicating copy to clipboard operation
bitrise-webhooks copied to clipboard

Rename `ShouldSkip`, for clarity of what this flag is meant to mean (should mean ~ Ignore This Event)

Open viktorbenei opened this issue 8 years ago • 1 comments

Related line: https://github.com/bitrise-io/bitrise-webhooks/blob/01ecc72252c22cc61541be694ed3dced0e485160/service/hook/common/common.go#L15

Should be renamed & add more comment.

This flag is meant to be used when the whole event should be discarded/skipped, with a success response. Examples:

  • GitHub sends a "ping" event when you add a new webhook, which can't be used and should not be used to trigger a build, it just have to be "acknowledged". Same is true for visualstudio.com / VSTS
  • Another case is that every service sends tag push events as "code commit" event. If you enable code push events for the webhook you'll get tag pushes too. Tag pushes are not handled by Bitrise.io right now, but returning an error every time would not be appropriate either, so we just acknowledge and skip it
    • Note: bitrise.io does support Tag triggers since

So the point is, ShouldSkip is too generic, the variable should be renamed to ShouldIgnoreEvent or similar for better clarity.

viktorbenei avatar Jun 07 '16 16:06 viktorbenei

Should also be noted that this will skip/ignore the whole event. E.g Bitbucket and VSTS can send commits in batches - if this flag is set then no build will be started, even if one commit could be used for triggering a build. This is a "full event" skip flag!

viktorbenei avatar Jun 07 '16 16:06 viktorbenei