search

dddeastanglia / DDDEastAnglia

DDD East Anglia website
https://www.dddeastanglia.com
7 stars 10 forks source link

Anonymous Sessions #326

Closed philpursglove closed 9 years ago

philpursglove commented 9 years ago

For feedback. (I have no idea why the commit from @adrianbanks is showing as part of this PR...)

alastairs commented 9 years ago

Probably that commit's not been merged to production. I'll take a look at the graph and see what's going on.

alastairs commented 9 years ago

Yeah, looks like the branch is based off staging rather than production. If you rebase, @adrianbanks' merge should disappear.

philpursglove commented 9 years ago

Ahh, will rebase off production this afternoon

philpursglove commented 9 years ago

Rebased to production, @adrianbanks' commit has gone :-)

alastairs commented 9 years ago

This looks ok to me. There is a lot of unrelated changes (whitespace, mostly), which would have been better split into a separate pull request for ease of diffing. I like to stick to the Single Responsibility Principle for pull requests, as it makes it much easier to deal with them.

(See also 10 Tips for Better Pull Requests)