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

feat: use currentBranch for diff if it has already been pushed to remote #30

Conversation

ericrallen
Copy link
Contributor

This PR introduces a check to see if the currentBranch has already been pushed to the remote. If it has, we will use it for the diff instead of the baseBranch.

Closes #26

@ericrallen
Copy link
Contributor Author

@theenadayalank Based on work in #31 I'm updating this PR to use the remote currentBranch instead of the local currentBranch using a similar pattern.

@ericrallen ericrallen force-pushed the feature/check-remote-branch branch from c474f6d to 3ab8bfa Compare June 27, 2019 00:37
@@ -3,7 +3,7 @@ const { execChildProcess } = require("./common");
module.exports = function fetchGitDiff( baseBranch = "master" ) {

// git command to pull out the changed file names between current branch and base branch (Excluded delelted files which cannot be fetched now)
let command = `git diff --relative --name-only --diff-filter=d ${baseBranch}...HEAD`;
let command = `git diff --relative --name-only --diff-filter=d ${baseBranch}`;
Copy link
Owner

Choose a reason for hiding this comment

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

I couldn't get the motive here. Can you shed some light here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...HEAD wasn't cooperating with the remote branch.

Copy link
Owner

Choose a reason for hiding this comment

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

IMO, it should. Can you explain it in more detail for further clarification?

Copy link
Owner

Choose a reason for hiding this comment

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

I have tried a few use cases with ...HEAD, it is working as intended for me.

Copy link
Owner

Choose a reason for hiding this comment

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

I think this PR is good to go after resolving this discussion 🔜 . Please make sure that the PR doesn't have any conflicts with the master 😉

@theenadayalank
Copy link
Owner

Closing this PR. Hence, the commits have been moved to #66

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.

Necessity of base branch
2 participants