Skip to content

Adds notification to offer PR create on publish#2362

Merged
RMacfarlane merged 4 commits into
masterfrom
feature/pr-notify
Jan 25, 2021
Merged

Adds notification to offer PR create on publish#2362
RMacfarlane merged 4 commits into
masterfrom
feature/pr-notify

Conversation

@eamodio

@eamodio eamodio commented Jan 15, 2021

Copy link
Copy Markdown
Contributor

When a branch is published to a remote, we will show a notification and ask if the user wants to create a new pull request.

image

Comment thread src/extension.ts

context.subscriptions.push(git.onDidPublish(async e => {
// Only notify on branch publish events
if (!e.branch || PersistentState.fetch(PROMPTS_SCOPE, PROMPT_TO_CREATE_PR_ON_PUBLISH_KEY) === false) {

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.

I think we should also avoid showing this notification if you're already in the create view - if you don't have an upstream for you current branch, we show a button that will run the publish action from there.

You could move this to reviewManager to have access to the CreatePullRequestHelper there, and also direct access to the create method instead of executing the command. CreatePullRequestHelper doesn't expose anything right now that indicates if create is in progress, so that would need to be added

@RMacfarlane RMacfarlane 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.

LGTM! just has a couple of linter warnings

Comment thread src/extension.ts Outdated
return;
}

const reviewManager = reviewManagers.find(reviewManager => reviewManager.repository.rootUri.toString() === e.repository.rootUri.toString());

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.

there are a couple of tslint warnings here, reviewManager is a shadowed variable and some missing semicolons below

@RMacfarlane RMacfarlane merged commit fe9636c into master Jan 25, 2021
@RMacfarlane RMacfarlane deleted the feature/pr-notify branch January 25, 2021 18:51
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.

2 participants