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

fix(3225): organize the return values of _parseHook [2] #61

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

y-oksaku
Copy link
Contributor

@y-oksaku y-oksaku commented Oct 21, 2024

Context

Currently, the _parseHook method returns Object or null or throw Error.
However, their return values are not properly conditioned.

Objective

Organize the return values by the following:

  • return Object (parsed result): the scm is correct and Screwdriver do actions with the webhook (e.g. push event)
  • return null: the scm is correct and Screwdriver do nothing (e.g. delete event)
  • throw Error: the scm is not correct

Then we can handle whether the webhook is correct or not at here.

References

Issue

PR

License

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@y-oksaku y-oksaku changed the title [2] fix(3225): organize the return values of _parseHook fix(3225): organize the return values of _parseHook [2] Oct 21, 2024
Copy link
Member

@tkyi tkyi left a comment

Choose a reason for hiding this comment

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

@y-oksaku
Copy link
Contributor Author

Thank you! I fixed it.

@tkyi tkyi merged commit f30ba16 into screwdriver-cd:master Oct 21, 2024
2 checks passed
@sd-buildbot
Copy link

🎉 This PR is included in version 4.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@y-oksaku y-oksaku deleted the fix-parse-hook branch October 22, 2024 00:03
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.

4 participants