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(middlware): allow string payloads #799

Merged
merged 5 commits into from Jan 12, 2023
Merged

Conversation

wolfy1339
Copy link
Member

Should allow #775 to be fixed in Probot without needing a new major version


Behavior

Before the change?

  • The payload was expected to be passed in request.body as a JSON object

After the change?

  • The payload can be either a string or a JSON object

Other information


Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • N/A

Pull request type

Please add the corresponding label for change this PR introduces:

  • Bugfix: Type: Bug
  • Feature/model/API additions: Type: Feature
  • Updates to docs or samples: Type: Documentation
  • Dependencies/code cleanup: Type: Maintenance

@wolfy1339 wolfy1339 added the Type: Feature New feature or request label Jan 12, 2023
@ghost ghost added this to Features in JS Jan 12, 2023
@wolfy1339 wolfy1339 changed the title Allow string middleware feat(middlware): allow string payloads Jan 12, 2023
@wolfy1339
Copy link
Member Author

Not sure how we could add tests for this, or if they would really be necessary

Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

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

I don't think trying to shoehorn in tests for this would be worth it; my 2 cents - I looked at the logic, and it LGTM. Thank you for doing this @wolfy1339 ❤️

@wolfy1339 wolfy1339 merged commit 4a9afd7 into main Jan 12, 2023
JS automation moved this from Features to Done Jan 12, 2023
@wolfy1339 wolfy1339 deleted the allow-string-middleware branch January 12, 2023 17:25
@github-actions
Copy link
Contributor

🎉 This PR is included in version 10.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link
Contributor

🎉 This PR is included in version 11.0.0-beta.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
JS
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants