Skip to content

Conversation

@LitchfieldACT
Copy link

added repeated start functionality by fully exposing TWI request bytes

added repeated start functionality by fully exposing TWI request bytes
@matthijskooijman
Copy link
Collaborator

I haven't looked at the actual code (I'm not familar with the sam target). However, I have a few comments regarding the form of your pull requests:

  • You provide two pullrequests, one for Wire.cpp and one for Wire.h. However, changes should be committed grouped as a logical change, it doesn't really matter in which files you change things. In particular, neither of your commits are now useful without the other, which is a sign that they should really be a single commit.
  • Your commit messages are focusing on the files (the summary line only says "Update Wire.cpp/h"), while they should be focusing on the actual functionality implemented. In particular, I think that using the second line in the commit as the summary line would be fine. You might want to include a bit more details about the change in the rest of the commit message. For example, it's not clear to me how exposing all the request bytes allows using repeated start, so that is something you could explain in the commit message (but perhaps also in comments in the code, of course).

Perhaps it is best if you create a new single pullrequest with the above comments kept in mind?

@matthijskooijman matthijskooijman mentioned this pull request Oct 14, 2013
@ffissore ffissore closed this Dec 3, 2013
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