sugarlabs / rpi23-gen-image

Advanced Debian "jessie" and "stretch" bootstrap script for RPi2/3
GNU General Public License v2.0
0 stars 1 forks source link

Merge for gsoc #13

Closed Rishabh42 closed 7 years ago

Rishabh42 commented 7 years ago
Rishabh42 commented 7 years ago

Should we use the 'rebase and merge' option from the drop down list next to the 'merge pull request' option to merge the PR instead of manually rebasing using the terminal?

quozl commented 7 years ago

Sqash and merge, rebase and merge, these are not the problem. The main problem is you are making changes without explaining them properly in commit messages attached to the change,

You did ask for review.

Please read again Making Commits. Note the ways in which your commits have not followed Sugar Labs contribution guidelines. As soon as I hit something I don't understand in your changes, I've gone looking for the explanation in the commit message and been disappointed.

Please review GitHub's collapsed view of the commits by clicking on the files changed tab of the pull request. Note how there are several changes in that view which are not explained in the commit messages.

You should try rebasing, and for safety do so in a separate branch, assuming your default remote is this repository;

git checkout -b gsoc-rebase
git fetch
git rebase --interactive

Whether you rebase into your own repository on GitHub or your own repository on a computer or phone, doesn't matter. I don't see why you haven't done that except that you haven't got any mentor who can explain it to you well, or you haven't spent the time to learn git.

I'll move comments around a bit in this pull request in a moment, and respond to your latest commit b85e1ca.

Rishabh42 commented 7 years ago

Thanks for informing me that I should explain each change I make, will keep that in mind while making changes going forward.