-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
|
||
Command = command; | ||
//if (command == Command) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove braces to reduce nesting. If the if-statement is not necessary, remove its comment as well.
@brookpatten this is really nice work, and I really appreciate the PR. One general comment, it appears that there are quite a bit of white space issues in the PR (things indented where they shouldn't be and vice verse). I would suggest trying to clean those up simply ot make the code more readable. I have attached several suggestions to the PR. I saw no major issues, so these comments really are suggestions. You have all been given contributor access to the repo, once you have reviewed my comments feel free to merge this PR. |
Good points all, I will make those changes and push it again. I knew there was going to be some carnage due to the size of the merge and how much time I spent just fiddling with things trying to get stuff working. I need to double check, but I believe in libpebble2 blobdb and install services are specified such that you're really not supposed to have more than 1 instance, I assume because the pebble only keeps track of 1 "token" at a time, so perhaps I need to rethink those a bit, possibly at a future date. |
I think that's everything, let me know if you're OK with it and I'll merge it. I have a few more changes in mind but I'll do them in the normal fashion as individual atomic PRs. |
Go ahead and merge it. |
This PR contains everything I've done in "Core" for the last year or so. Sorry for the huge list, but it would take forever to split everything out in to separate PRs. I will break things out smaller for future changes. Included is processing for firmware v3 bundles (including folder structure, manifest, resource files etc), AppMessage (send/receive), App installation & launching on firmware v3, (as well as installation and launching on v2, but that's obsolete now).
There were a few pre-existing build issues in the wpf library, due to a missing file, I attempted to replace it. There are also some preexisting build issues in windows universal. It could just be that I'm using a newer visual studio than they were created with, I'm not sure.
I also started breaking things out into separate classes rather than having everything inside Pebble. I was thinking long term it might be a good goal to have this lib mimic the structure of libpebble2, that way as changes occur there it should be relatively straightforward to do corresponding changes on PebbleSharp.
Once Mono releases 4.33+ and dbus-sharp accepts the file descriptor PR I will push the mono/bluez changes as well. For the time being though they wouldn't build without some hacky weirdness.