Opens all changes after open in worktree flow finished for PRs #3701

nzaytsev · 2024-10-28T04:20:42Z

Description

Screen.Recording.2024-10-28.at.11.18.31.mov

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

package.json

src/commands/git/worktree.ts

src/git/actions/worktree.ts

src/plus/launchpad/launchpadProvider.ts

src/uris/deepLinks/deepLinkService.ts

axosoft-ramint

I tried creating a VSIX from the latest of this branch, and running "open in worktree" on a few PRs, but I'm never able to see the PR changes opened. I will check if there was an issue with the package creation but in the meantime, some comments below:

src/commands/git/worktree.ts

src/plus/launchpad/launchpadProvider.ts

src/uris/deepLinks/deepLink.ts

src/uris/deepLinks/deepLinkService.ts

axosoft-ramint

I found out why I wasn't able to see the PR changes in my test cases using the Launchpad. See below.

Also have an overall suggestion for making this much simpler. Let me know if it would work or if we would run into issues.

src/views/viewCommands.ts

src/uris/deepLinks/deepLinkService.ts

Co-authored-by: Ramin Tadayon <[email protected]>

axosoft-ramint

This works for me - though I was speaking with @eamodio this morning and thinking that it might not make sense to always load the changes. Will follow up on that in a bit.

axosoft-ramint · 2024-11-01T18:26:55Z

src/uris/deepLinks/deepLink.ts

 		[DeepLinkServiceAction.OpenGraph]: DeepLinkServiceState.OpenGraph,
 		[DeepLinkServiceAction.OpenFile]: DeepLinkServiceState.OpenFile,
 		[DeepLinkServiceAction.OpenSwitch]: DeepLinkServiceState.SwitchToRef,
+		[DeepLinkServiceAction.OpenAllPrChanges]: DeepLinkServiceState.OpenAllPrChanges,


This transition is no longer necessary since it would/should only be reachable from OpenInspect

axosoft-ramint · 2024-11-01T18:29:17Z

src/uris/deepLinks/deepLinkService.ts

 						repository: repo,
 						source: 'launchpad',
 					} satisfies ShowWipArgs);
+					action = DeepLinkServiceAction.OpenAllPrChanges;


Right now this would work because we really only open inspect on PR switches, in which case all the info is there to show the PR changes. Would be a nice additional touch though to check the action type in this._context.action though and maybe restrict/guard against any non-PR case.

axosoft-ramint · 2024-11-01T22:00:45Z

Hey @nzaytsev, while reviewing your PR, I'd suggest the following code changes:

👉 Only open changes when worktree is created

Removes unneeded transition event

Only opens changes when using "Open in Worktree"
Only opens changes when a worktree is created in the flow

You can also review and apply these suggestions locally on your machine.

Learn more about GitKraken Code Suggest

Code Suggest liberates your code reviews from GitHub's restrictive, comment-only feedback style. As simple as suggesting changes in a Google-doc, provide real code suggestions from where you code, e.g. your IDE, and on anything in your project — not just on the lines of code changed in the PR.

Join your team on GitKraken to speed up PR review.

axosoft-ramint · 2024-11-01T22:05:22Z

Hey @nzaytsev, while reviewing your PR, I'd suggest the following code changes:

👉 Restricts to only opening on "Open in Worktree" and only when worktree is created

You can also review and apply these suggestions locally on your machine.

Learn more about GitKraken Code Suggest

Code Suggest liberates your code reviews from GitHub's restrictive, comment-only feedback style. As simple as suggesting changes in a Google-doc, provide real code suggestions from where you code, e.g. your IDE, and on anything in your project — not just on the lines of code changed in the PR.

Join your team on GitKraken to speed up PR review.

- Only shows changes for "open in worktree" action - Removes unnecessary state transition - Only shows changes if worktree is created in flow - Applies to branches in views and graph

axosoft-ramint · 2024-11-05T16:03:00Z

@eamodio Asking for your help reviewing this one since I committed to it directly.

axosoft-ramint · 2024-11-06T15:34:24Z

Going to go ahead and merge this - we can address further issues in follow-up

nzaytsev linked an issue Oct 28, 2024 that may be closed by this pull request

Open multi-diff editor when opening a new worktree from a PR #3549

Closed

nzaytsev requested review from axosoft-ramint and eamodio and removed request for axosoft-ramint October 28, 2024 04:20

axosoft-ramint requested changes Oct 29, 2024

View reviewed changes

nzaytsev force-pushed the features/3549-open-multi-diff-editor-when-opening-a-new-worktree-from-a-pr branch from dacb6c6 to 4c7014a Compare October 30, 2024 04:56

nzaytsev requested a review from axosoft-ramint October 30, 2024 05:30

axosoft-ramint requested changes Oct 30, 2024

View reviewed changes

axosoft-ramint requested changes Oct 31, 2024

View reviewed changes

src/views/viewCommands.ts Outdated Show resolved Hide resolved

src/uris/deepLinks/deepLinkService.ts Outdated Show resolved Hide resolved

src/uris/deepLinks/deepLinkService.ts Outdated Show resolved Hide resolved

src/uris/deepLinks/deepLinkService.ts Outdated Show resolved Hide resolved

nzaytsev and others added 2 commits November 1, 2024 14:51

Opens all changes after open in worktree flow finished for PRs

8ee240e

Update src/uris/deepLinks/deepLinkService.ts

861824b

Co-authored-by: Ramin Tadayon <[email protected]>

nzaytsev force-pushed the features/3549-open-multi-diff-editor-when-opening-a-new-worktree-from-a-pr branch from 02a3e5a to 861824b Compare November 1, 2024 07:52

axosoft-ramint reviewed Nov 1, 2024

View reviewed changes

Refinements

74a57e3

- Only shows changes for "open in worktree" action - Removes unnecessary state transition - Only shows changes if worktree is created in flow - Applies to branches in views and graph

axosoft-ramint self-requested a review November 5, 2024 15:17

axosoft-ramint approved these changes Nov 5, 2024

View reviewed changes

axosoft-ramint merged commit 41600ec into main Nov 6, 2024
3 checks passed

axosoft-ramint deleted the features/3549-open-multi-diff-editor-when-opening-a-new-worktree-from-a-pr branch November 6, 2024 15:55

Opens all changes after open in worktree flow finished for PRs #3701

Opens all changes after open in worktree flow finished for PRs #3701

Uh oh!

Conversation

nzaytsev commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

axosoft-ramint left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

axosoft-ramint left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

axosoft-ramint left a comment

Choose a reason for hiding this comment

Uh oh!

axosoft-ramint Nov 1, 2024

Choose a reason for hiding this comment

Uh oh!

axosoft-ramint Nov 1, 2024

Choose a reason for hiding this comment

Uh oh!

axosoft-ramint commented Nov 1, 2024

Uh oh!

axosoft-ramint commented Nov 1, 2024

Uh oh!

axosoft-ramint commented Nov 5, 2024

Uh oh!

axosoft-ramint commented Nov 6, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nzaytsev commented Oct 28, 2024 •

edited

Loading