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

Adding branch name to "bilt-with-bilt" git commit #70

Closed
wants to merge 4 commits into from

Conversation

zivkaziv
Copy link
Contributor

@zivkaziv zivkaziv commented May 8, 2021

Added the branch name to the git commit. So instead of having [bilt-with-bilt] it's now [bilt-with-bilt-_branchName_]

* @param {string} cwd
*/
async function getGitBranchName(cwd) {
const branchName = await shWithOutput(`git branch --show-current`, {
Copy link
Owner

Choose a reason for hiding this comment

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

In case of detached head (e.g. after git checkout HEAD~), the command git branch --show-current will return an empty string. I would suggest that in this case, we use the commit SHA.

@@ -322,7 +323,8 @@ async function addLastBuildTimeToPackageInfos(packageInfos, packageChanges, root
cwd: rootDirectory,
})

if (stdout.includes('[bilt-with-bilt]')) {
const branchName = await getGitBranchName(rootDirectory)
Copy link
Owner

Choose a reason for hiding this comment

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

This command is inside the "loop" (it's a map, so not really a loop). This means that it gets executed for each packageChange, where it could be executed outside of it all, and just once (as could be the string [bilt-with-bilt-${branchName}], which could be also calculated outside the loop.

@zivkaziv
Copy link
Contributor Author

Thanks for your feedbacks... I'll fix them by the EOW:)
Thanks

@zivkaziv
Copy link
Contributor Author

Hey @giltayar, sorry the it took me forever to work on it... I pushed those changes... Please let me know what do you think

@giltayar
Copy link
Owner

@zivkaziv after my experience working with branches and bilt, it is now my conviction that the way Bilt figures out whether a package was built or not is wrong, and that currently the only way to use Bilt is to do trunk-based development without PRs.

I am working with @reggev on figuring out how to restore this important capability, but till then, I don't think this PR makes any sense. I'm SO sorry you had to waste your time on this, but for now I'm closing the issue that this PR is based on.

@zivkaziv
Copy link
Contributor Author

No problem... It was fun:)
I'll submit another PR instead....

@zivkaziv zivkaziv closed this May 31, 2021
@zivkaziv zivkaziv deleted the 66-add-branch-name branch May 31, 2021 10:01
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