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

add active lock reason to PR #2543

Merged
merged 6 commits into from
Aug 19, 2022

Conversation

notauserx
Copy link
Contributor

@notauserx notauserx commented Aug 12, 2022

add active lock reason to PR.
Since every pull request is an issue and the lock and unlock are on the IIssuesClient, updates the docs to reflect the design.
Fixes #2248

create ILockUnlockClient so both IIssuesClient and IPullRequestClient can
access lock and unlock methods.
@notauserx
Copy link
Contributor Author

notauserx commented Aug 16, 2022

I have extracted the lock/unlock methods from IIssuesClient to a new ILockUnlockClient, which is available in both the IIssuesClient and IPullRequestClient, so both clients can use the ILockUnlockClient.

I need a review for this PR and any feedback. I have intentionally left out the refactoring on the Reactive project, I will get to that once I get a review.

@JonruAlveus
Copy link
Collaborator

Hi @notauserx, thanks for this contribution ♥️! What you’ve done looks great, there’s just a couple of points:

  • Please check the pr for warnings raised during the build as we’re trying to keep the warnings to none if possible
  • It looks like the build is failing, but I think you’ve got that covered by the upcoming Reactive changes
  • If you have time (and the desire!), please could you add/update the readme md files in the root/docs folder and/or any related linqpad samples?

@notauserx
Copy link
Contributor Author

Thanks @JonruAlveus for the feedback. I will also update the relevant readme and the linqpad samples in the PR.

@notauserx notauserx changed the title [WIP] add active lock reason to PR add active lock reason to PR Aug 17, 2022
@notauserx
Copy link
Contributor Author

@JonruAlveus I have updated the docs in the Issues.md. I did not find any relevant linqpad samples to update. Please let me know what you think.

Copy link
Collaborator

@JonruAlveus JonruAlveus left a comment

Choose a reason for hiding this comment

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

Super happy with everything you’ve done here, if you can fix up the warnings I’d be happy to approve for merge!

@JonruAlveus
Copy link
Collaborator

release_notes: Reworks how Locking and Unlocking works on Issues and adds Locking and Unlocking to Pull Requests

@JonruAlveus JonruAlveus merged commit 595e35d into octokit:main Aug 19, 2022
@notauserx notauserx deleted the add-active-lock-reason-to-pr branch August 20, 2022 05:43
@nickfloyd nickfloyd added Type: Breaking change Used to note any change that requires a major version bump Type: Feature New feature or request and removed category: breaking-change labels Oct 26, 2022
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: Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue active_lock_reason missing
3 participants