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

cakeArguments must be array #80

Merged
merged 1 commit into from
Oct 24, 2019

Conversation

Roadrunner67
Copy link
Contributor

Initializes $cakeArguments as array instead of string.

$($cakeArguments -join " ") will not insert spaces between arguments if it is a string.

build.ps1 Outdated Show resolved Hide resolved
build.ps1 Outdated Show resolved Hide resolved
build.ps1 Outdated Show resolved Hide resolved
@jnm2
Copy link
Contributor

jnm2 commented Oct 21, 2019

@devlead I just picked up the buggy version in a PR to an OSS project and my PR was merged, so now they have to deal with that. I think this repo is referenced fairly frequently, so the less time for the buggy version to spread, the better.

@xt0rted
Copy link

xt0rted commented Oct 24, 2019

Just hit this while setting a new project up. Fixed it with $cakeArguments = @() and things seem to be back to normal again.

@Roadrunner67
Copy link
Contributor Author

Can anyone explain how to squash my commits into one, when there is a merge develop into my issue branch in the middle (stupid level explanation needed)?

@jnm2
Copy link
Contributor

jnm2 commented Oct 24, 2019

The way I would do it is to do git reset <base commit> which will roll back the history and make <base commit> the current commit without changing any of your files.

The parent of your first commit is ce57fc7. To roll back to there without touching your files:

git reset ce57fc7

Then to stage all the changes in your files since ce57fc7:

git add *

Then to create a new commit with all these changes:

git commit -m "Some message"

Double-check your local commit history before pushing just so there are no surprises. You should expect to see a single commit with your message and the exact changes you want to see in this PR, and its parent commit should be ce57fc7.

Then to update the PR branch, since you'd be overwriting history:

git push --force

Force pushes are pretty safe since GitHub shows the entire history in the PR timeline, including the original commit hashes in case they are needed, and no one else is working in your branch.

Then, there will be a single commit in the Commits section of this PR.

@Roadrunner67
Copy link
Contributor Author

Roadrunner67 commented Oct 24, 2019

Thanks @jnm2
The local commit DOES have ce57fc7 as parent, but at the same time it includes ALL the changes that have been committed after ce57fc7, mostly done by other contributers.

That is, of course, on my own forked repo.

Will it look right if pushed, or should I kill this PR and build a new from a new fork?

@jnm2
Copy link
Contributor

jnm2 commented Oct 24, 2019

Oh, good catch. You want to get rid of the merge commit before doing this. Interactive rebase is the fastest way to drop a commit that far back, but interactive rebase isn't easy unless you have Notepad++ or VS Code or something set up as your git text tool.

To undo my bad instructions and get your local branch matching this branch again, do git reset origin/issue/74+78-take-2 or git reset d1030d6e. (The first one assumes that the name of the remote for your fork is origin.)

Starting over, this is what I'd do if I didn't use interactive rebase.
Create a backup branch in case things go sideways:

git branch foo_bak

Reset back to the branch which you want to be the parent commit. You could make this a newer commit from upstream/develop but let's go with ce57fc7, the current parent, just for simplicity. This time, use --hard to throw away all outstanding changes after switching to ce57fc7:

git reset --hard ce57fc7

Then replay the commits you want to keep, one by one. --no-commit means the changes will pile up without committing, so the --no-commit is letting you squash while you do this instead of ending up with individual commits. If you didn't want to squash, simply omit the --no-commit parameter.

git cherry-pick --no-commit 2ae38f26 (hash from 'cakeArguments must be array')
git cherry-pick --no-commit 51db921a (hash from 'Typo: If -> if')
git cherry-pick --no-commit f123495b (hash from 'Comments')
git cherry-pick --no-commit d1030d6e (hash from 'Concatenating arrays (again)')

Finally, git commit -m 'Some message' and double check that it looks good and then git push -f.

git branch -D foo_bak once you don't need it.

@Roadrunner67
Copy link
Contributor Author

I think it looks good now.
I guess I should merge develop into my branch now as suggested by Github?

@jnm2
Copy link
Contributor

jnm2 commented Oct 24, 2019

@Roadrunner67 You can. My preference for my own branches is to rebase instead of merging. Rebasing replays your set of commits (you have just one now) on top of a different parent commit, one by one, stopping to allow merge conflict resolution for each one that needs it if any.

If you don't need to fetch to get the latest commits from upstream/develop, this is all you have to do:

git rebase upstream/develop

If you want to fetch and rebase in a single command:

git pull -r upstream develop

(-r is for --rebase. Some people make rebasing the default with a git config setting so that we don't have to type -r after pull.)

Rebasing does mean rewriting history, so the usual caveats apply. Only use in branches that no one else is working in, and check carefully before doing git push -f.

Keep cakeArguments as array (not string) and only add Script (quoted) if Script is not empty.
@Roadrunner67
Copy link
Contributor Author

Got it, history is so much cleaner now with the rebase.
In parallel to learning git from cli, I also use Fork - it's just a great visualizer of what's going on.
Thank you so much for your kind and patient help, I hope I will be able to give some back with future contributions.
Hopefully, this PR can be closed soon.

@jnm2
Copy link
Contributor

jnm2 commented Oct 24, 2019

You're very welcome! As another Cake user, thanks for these improvements to Cake!

I love Fork! I just started using it. Its interactive rebase UI is so good that I debated suggesting that you use it earlier in the thread. Oh well :D I learned on the CLI and typing is faster than clicking, but Fork is a beautiful piece of software.

Copy link
Member

@devlead devlead left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@devlead devlead merged commit d321f2d into cake-build:develop Oct 24, 2019
@devlead
Copy link
Member

devlead commented Oct 24, 2019

@Roadrunner67 your changes have been merged, thanks for your contribution 👍

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.

4 participants