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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 60 additions & 19 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,30 +56,71 @@ if (process.stdout.isTTY) {
return;
}

// Fetching committed git files
fetchGitDiff( baseBranch ).then((committedGitFiles = []) => {
debug(committedGitFiles);
new Listr(resolveMainTask({ tasks, committedGitFiles }), {
exitOnError: false,
concurrent: true,
collapse: false
// see if the branch exists on the remote
// adapted from: https://stackoverflow.com/a/44401325/656011
const isCurrentBranchPublished = `git ls-remote --exit-code --heads $(git remote | head -1) "${currentBranch}"`;

let resolveBranch = Promise.resolve(baseBranch);

// we're going to use the success/error of the call to isCurrentBranchPublished
// so we'll return the correct branch based on the exit code we receive
const getDiffBranch = execChildProcess({ command: isCurrentBranchPublished })
.then(() => {
const getRemote = 'git remote | head -1';

// if the current branch has been pushed to the remote
// we'll compare to the currentBranch for our diff
return execChildProcess({ command: getRemote })
.then((remote) => {
const branch = `${remote}/${currentBranch}`;
debug('Branch to Diff:' + branch);
return branch;
})
.catch(() => {
debug('Branch to Diff:' + baseBranch);

// if we can't find the remote, we'll fall back to the baseBranch
return baseBranch;
});
})
.run()
.then(() => {
cache.setSync("linted-hash", commitHash);
debug('Cached Current Commit Hash');
log(success("\nVoila! 🎉 Code is ready to be Shipped.\n"));
.catch(() => {
debug('Branch to Diff:' + baseBranch);

return resolveBranch;
});

// now that we have the branch we need to diff against, let's diff
getDiffBranch.then((diffBranch) => {
// Fetching committed git files
fetchGitDiff( diffBranch ).then((committedGitFiles = []) => {
debug(committedGitFiles);
new Listr(resolveMainTask({ tasks, committedGitFiles }), {
exitOnError: false,
concurrent: true,
collapse: false
})
.catch(({ errors }) => {
process.exitCode = 1;
errors.forEach(err => {
console.error(err.customErrorMessage);
.run()
.then(() => {
cache.setSync("linted-hash", commitHash);
debug('Cached Current Commit Hash');
log(success("\nVoila! 🎉 Code is ready to be Shipped.\n"));
})
.catch(({ errors }) => {
process.exitCode = 1;
errors.forEach(err => {
console.error(err.customErrorMessage);
});
});
});
})
.catch((message = '') => {
process.exitCode = 1;
log(warning(message));
});
})
.catch((message = '') => {
.catch((e) => {
log(error(e));
process.exitCode = 1;
log(warning(message));
return;
});
});
})
Expand Down
2 changes: 1 addition & 1 deletion utils/fetchGitDiff.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 😉


return new Promise( (resolve, reject) => {
return execChildProcess({ command })
Expand Down