Skip to content

[Breaking change] fix/update call to delete_reaction#1588

Merged
nickfloyd merged 6 commits into
octokit:mainfrom
dimerman:main
Jul 19, 2023
Merged

[Breaking change] fix/update call to delete_reaction#1588
nickfloyd merged 6 commits into
octokit:mainfrom
dimerman:main

Conversation

@dimerman

@dimerman dimerman commented Jun 29, 2023

Copy link
Copy Markdown
Contributor

BREAKING CHANGE

Couldn't find an open issue. Instead of opening a new one, I just fixed it.


Behavior

Before the change?

  • reactions created could not be deleted.

After the change?

  • reactions created could be deleted.

Other information

  • I don't think this was properly tested

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

@nickfloyd nickfloyd added the Type: Bug Something isn't working as documented label Jul 18, 2023

@nickfloyd nickfloyd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @dimerman thank you for the contributions here! Just a few comments about the state of thing on the GitHub REST API surface. Since this is effectively a breaking change / or one where the method didn't even work I'd be inclined to even change the method name as well to more appropriately match what it does. Let me know your thoughts.

Comment thread lib/octokit/client/reactions.rb Outdated
Comment thread lib/octokit/client/reactions.rb Outdated
dimerman and others added 2 commits July 18, 2023 10:54
Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com>
Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com>
@dimerman

Copy link
Copy Markdown
Contributor Author

👍🏻 LGTM

@dimerman dimerman requested a review from nickfloyd July 18, 2023 17:55
nickfloyd
nickfloyd previously approved these changes Jul 18, 2023

@nickfloyd nickfloyd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the contributions ❤️

@nickfloyd nickfloyd added the Type: Breaking change Used to note any change that requires a major version bump label Jul 18, 2023
@nickfloyd nickfloyd self-requested a review July 18, 2023 17:59
@nickfloyd nickfloyd dismissed their stale review July 18, 2023 18:00

needs tests to be updated

@dimerman

Copy link
Copy Markdown
Contributor Author

@nickfloyd just fixed the specs + moved the VCR cassette 👍🏻

@nickfloyd nickfloyd changed the title fix call to delete_reaction [Breaking change] fix/update call to delete_reaction Jul 19, 2023
@nickfloyd nickfloyd merged commit 54c1e6d into octokit:main Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Breaking change Used to note any change that requires a major version bump Type: Bug Something isn't working as documented

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants