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

clock: Replace [<TestCase>] tests with individual tests (#298) #301

Merged
merged 8 commits into from
Feb 9, 2017
Merged

clock: Replace [<TestCase>] tests with individual tests (#298) #301

merged 8 commits into from
Feb 9, 2017

Conversation

balazsbotond
Copy link
Contributor

See #298

@robkeim
Copy link
Contributor

robkeim commented Feb 9, 2017

Great work, thanks @balazsbotond!

@robkeim robkeim merged commit 5e18ac6 into exercism:master Feb 9, 2017
@robkeim
Copy link
Contributor

robkeim commented Feb 9, 2017

@balazsbotond I've noticed that all of your PRs tend to have a lot of unrelated changes that have already been merged. I've been merging them with squash commits so it doesn't show up in the history (except the extended description). This one for example has 8 commits. Do you know why that is?

@balazsbotond
Copy link
Contributor Author

@robkeim I don't know exactly why but I think if I delete my fork, create a new one, develop all new changes in separate branches and delete them after the pull request has been merged, and then update my fork with --ff-only, the issue will be resolved. I'll try it with the next pull request.

@balazsbotond balazsbotond deleted the clock-298 branch February 9, 2017 16:55
@robkeim
Copy link
Contributor

robkeim commented Feb 9, 2017

You don't need to delete your fork, you just need to make a pull from the upstream branch to keep your fork up-to-date.

Here's the script that I use to update my fork:

git fetch upstream
git checkout master
git merge upstream/master
git push

Once your fork is up-to-date, then you can develop your changes from the head of master. Does that make sense?

@balazsbotond
Copy link
Contributor Author

Thanks. I did the exact same thing, so I have no idea what went wrong...

@robkeim
Copy link
Contributor

robkeim commented Feb 9, 2017

Hmm... I'm not sure then. Did you rebase on to master after updating your fork?

I'm not a git wizard so I can't tell you all of the commands to make sure that things look right, but I use SourceTree to visualize the source history and make sure everything looks right before sending my PRs.

@ErikSchierboom
Copy link
Member

About the Git stuff, I also noticed that. It's not really a problem due to us being able to squash the commits, however it does look a bit odd. How I usually approach this, is by doing a rebase on my branch.

So suppose I have the branch checked out that I will submit as a PR. I then do the following:

git fetch upstream
git rebase upstream/master
git push --force

This will rebase the branch based on the latest master branch of the upstream remote. You then usually have to do force push, as the tree has become different. This is no problem, as long as you are pushing the branch to your own fork (which I assume you are the only one working on).

Note that if you use the GitHub desktop client to clone the repository, the original remote (from which you forked) will be named exercism. So in that case, exercism replaces upstream.

@ErikSchierboom
Copy link
Member

And thanks for the PR @balazsbotond! Hopefully @kytrinyx will soon be able to add you as a maintainer.

@balazsbotond
Copy link
Contributor Author

Thanks @ErikSchierboom, this solution looks very promising - I'll try it for my next pull request :)

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.

3 participants