cake-build / resources

Contains different kind of resources such as bootstrappers and configuration files.
MIT License
54 stars 78 forks source link

cakeArguments must be array #80

Closed Roadrunner67 closed 5 years ago

Roadrunner67 commented 5 years ago

Initializes $cakeArguments as array instead of string.

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

jnm2 commented 5 years ago

@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 commented 5 years ago

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

Roadrunner67 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

jnm2 commented 5 years ago

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

Roadrunner67 commented 5 years ago

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 commented 5 years ago

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.

devlead commented 5 years ago

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