Skip to content

Conversation

@sergeibbb
Copy link
Member

@sergeibbb sergeibbb commented Feb 25, 2025

Removes SearchedIssue and SearchedPullRequest wrappers because reasons are not used anymore (#4098)

Checklist

  • I have followed the guidelines in the Contributing document
  • My changes follow the coding style of this project
  • My changes build without any errors or warnings
  • My changes have been formatted and linted
  • My changes include any required corresponding changes to the documentation (including CHANGELOG.md and README.md)
  • My changes have been rebased and squashed to the minimal number (typically 1) of relevant commits
  • My changes have a descriptive commit message with a short title, including a Fixes $XXX - or Closes #XXX - prefix to auto-close the issue that your PR addresses

@sergeibbb sergeibbb linked an issue Feb 25, 2025 that may be closed by this pull request
3 tasks
@sergeibbb sergeibbb force-pushed the debt/4098-remove-searched-wrappers branch from 78da759 to 3cb128e Compare February 25, 2025 13:42
sergeibbb added a commit that referenced this pull request Feb 25, 2025
sergeibbb added a commit that referenced this pull request Feb 25, 2025
@sergeibbb sergeibbb force-pushed the debt/4098-remove-searched-wrappers branch from 3cb128e to 316b018 Compare February 25, 2025 13:46
Copy link
Contributor

@axosoft-ramint axosoft-ramint left a comment

Choose a reason for hiding this comment

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

On a scan., LGTM.

Lots of touched areas to test here - would be easier in the pre-release.

filterUsed?: IssueFilter,
userLogin?: string,
): SearchedIssue | undefined {
export function toSearchedIssue(issue: ProviderIssue, provider: ProviderReference): IssueShape | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

Probably to rename to something like:

Suggested change
export function toSearchedIssue(issue: ProviderIssue, provider: ProviderReference): IssueShape | undefined {
export function toIssueShape(issue: ProviderIssue, provider: ProviderReference): IssueShape | undefined {

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@eamodio eamodio left a comment

Choose a reason for hiding this comment

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

Looks good to me too -- 1 minor nit

sergeibbb added a commit that referenced this pull request Feb 26, 2025
sergeibbb added a commit that referenced this pull request Feb 26, 2025
@sergeibbb sergeibbb force-pushed the debt/4098-remove-searched-wrappers branch from 316b018 to 867f9d2 Compare February 26, 2025 08:21
@sergeibbb sergeibbb force-pushed the debt/4098-remove-searched-wrappers branch from 867f9d2 to 36cc563 Compare February 26, 2025 08:24
@sergeibbb sergeibbb added the needs-verification Request for verification label Feb 26, 2025
@sergeibbb
Copy link
Member Author

sergeibbb commented Feb 26, 2025

@axosoft-ramint

Lots of touched areas to test here - would be easier in the pre-release.

I've added testing notes to #4098

@sergeibbb sergeibbb merged commit 56a5fa2 into main Feb 26, 2025
3 checks passed
@sergeibbb sergeibbb deleted the debt/4098-remove-searched-wrappers branch February 26, 2025 08:42
@sergeibbb sergeibbb removed the needs-verification Request for verification label Feb 26, 2025
saeedzaha pushed a commit to saeedzaha/vscode-gitlens that referenced this pull request Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of SearchedIssue and SearchedPullRequest is no longer neccesary

3 participants