-
Notifications
You must be signed in to change notification settings - Fork 905
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
(GH-460) Fix Uninstall Template $key array #462
Conversation
This is really good. As a further enhancement, I would line break your git commit message body around 72 chars. And update your summary a bit - you are fixing the uninstall template. It's okay to use |
I appreciate your patience on this Rob. Until I got the hang of Github and how it all works I have been working with the Windows GUI. While the command line is not alien to me at all (using PowerShell on a daily basis) working with Github through it is! I have updated my commit text and wrapped it at 72 (that was in your doc - I forgot about it) but I'm not sure if I have to remove my pull request and resubmit? |
Perfect! Once AppVeyor is happy, we'll move forward. |
Once the PR is repushed, everything shows up here again for commit validation checks. |
I want to smile when I ask but ... how to I repush a PR? I'm Googling. I am. But I cant find anything that tells me. |
git push origin fixup_uninstall_template:master http://stackoverflow.com/questions/8479559/github-pushing-to-pull-requests ? |
@pauby we actually wrote it all up for you in https://github.com/chocolatey/choco/blob/master/CONTRIBUTING.md#respond-to-feedback-on-pull-request |
When you make additional commits, you can use |
The update of the branch will automatically update the existing PR. |
@pauby Whoopsy. Don't ever use |
Rob, apologies. I'm actually at a loss on how to proceed any further. The instructions provided didn't work and Git itself suggested a git pull which is why I used it. |
Mostly you should always use |
Then read over the instructions again. When you fetch and rebase master (which wasn't required here), you do that against the master branch, not your feature branch. |
Rob, when I follow the instructions I get the following errors: git push origin fixup_uninstall_template Probably something to do with what I have already done. I really don't want to do anymore in case I make things worse. |
Oh, and I have installed Git Extensions. Just looking at it now. |
Thats why -f, which is in the instructions. On Friday, October 16, 2015, Paul Broadwith [email protected]
Rob http://devlicio.us/blogs/rob_reynolds |
Used -f for the push now. |
So you are going to likely need to back off the merge from master commit, then do another force push. Optionally I can cherry pick your commits and fix it up if you feel like you've gotten in too big of a mess to get out of. Hope you are looking at this as a learning opportunity, not something that is scary. Git is quite powerful and it can be quite daunting for newcomers. |
These are very specific instructions to follow, and should be followed almost near exactly as they are specified.
|
Totally agree with this. Although I am "comfortable" with git, I know there is still a LOT I don't know about it. But once you grasp the basics, you will be amazed at what you can achieve. |
(GH-460) Amended Template Using 'choco new' to produce ChocolateyUninstall.ps1 for your package would result in the uninstall block never being executed as the $key variable type was wrong. Updated the script template to force the $key variable to always be an array and therefore have a Count method that can be evaluated and the uninstall block executed. Closes #460
@pauby that's really close. :) |
Lol. Thanks Rob. Close but not close enough? Why is it failing? |
TeamCity always fails. The reason I said it was close but not close enough. Your commit was squashed into an already pushed commit. |
@pauby I'll take this one for you and fix it up. No worries. |
Thanks for working with me on this. I hope that so far it's a good learning opportunity! |
Thanks Rob. Let's just say it's been an ... experience :) |
@pauby if you want to pair up on this, I'd be open to it. |
@ferventcoder What did you have in mind? |
contact me at ferventcoder gmail. |
Using 'choco new' to produce ChocolateyUninstall.ps1 for your package would result in the uninstall block never being executed as the $key variable type was wrong. Updated the script template to force the $key variable to always be an array and therefore have a Count method that can be evaluated and the uninstall block executed.
Closes #460
Supersedes #461