servalproject / serval-dna

The Serval Project's core daemon that implements Distributed Numbering Architecture (DNA), MDP, VoMP, Rhizome, MeshMS, etc.
http://servalproject.org
Other
171 stars 80 forks source link

RFM69 radio link integration #87

Open muebau opened 10 years ago

lakeman commented 10 years ago

It looks like you're exclusively using TAB characters for indentation. We've been using the convention of indenting by 2 spaces, with TAB's representing 8 spaces. This difference makes it hard to see what you have actually changed.

Also it looks like you could easily rebase your last commit onto our development branch without needing to add any unnecessary merges into our commit history.

muebau commented 10 years ago

Hello, what I would do:

  1. create a new (temp) branch from development.
  2. cherry pick my changes
  3. fix the formatting
  4. reset (--hard) the development branch to the (temp) branch
  5. send another pull request

For the future I would do "git pull --rebase" to avoid the problem in the future.

Would this help? Do you have any suggestions of improvement?

lakeman commented 10 years ago

git pull --rebase and git rebase -i [remote name]/development

Just create new commits until you think the current version is right, then use rebase -i to squash everything that you think should be combined into one patch. While the end result is the same as reset --hard and / or cherry-picking, rebase -i is a bit easier to control without losing changes IMHO.

On Tue, Oct 7, 2014 at 5:43 PM, muebau notifications@github.com wrote:

Hello, what I would do:

  1. create a new (temp) branch from development.
  2. cherry pick my changes
  3. fix the formatting
  4. reset (--hard) the development branch to the (temp) branch
  5. send another pull request

For the future I would do "git pull --rebase" to avoid the problem in the future.

Would this help? Do you have any suggestions of improvement?

— Reply to this email directly or view it on GitHub https://github.com/servalproject/serval-dna/pull/87#issuecomment-58145666 .