Skip to content

GTOID improvements #206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 9, 2013
Merged

GTOID improvements #206

merged 11 commits into from
Jul 9, 2013

Conversation

5sw
Copy link
Contributor

@5sw 5sw commented Jul 8, 2013

No description provided.

@@ -9,7 +9,7 @@
#import "git2.h"

// Represents an object ID.
@interface GTOID : NSObject
@interface GTOID : NSObject < NSCopying >
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: no spaces on either side of NSCopying.

@joshaber
Copy link
Member

joshaber commented Jul 8, 2013

Awesome, thanks! Just some style notes.

@@ -32,6 +32,10 @@
NSString *secondSHA = @"82dc47f6ba3beecab33080a1136d8913098e1801";
expect(testOID).notTo.equal([[GTOID alloc] initWithSHA:secondSHA]);
});

it(@"should compare equal to an OID created with the same SHA from a C string", ^{
expect(testOID).to.equal([[GTOID alloc] initWithSHACString: "f7ecd8f4404d3a388efbff6711f1bdf28ffd16a0"]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: no space after colon.

@5sw
Copy link
Contributor Author

5sw commented Jul 9, 2013

I think now I am done with the GTOID class.

I started another branch (https://github.com/5sw/objective-git/tree/use_gtoid) where I change the use of the plain git_oid structure to the GTOID class. Seems to me there is a lot of duplicated code all around that converts between git_oid and strings.

// Initializes the receiver by converting the given SHA to an OID
// optionally returning a NSError instance on failure.
//
// SHA - The to convert to an OID. Cannot be nil.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: align the -'s

@dannygreg
Copy link
Contributor

Loving this work @5sw! It would be great to have some unit tests for the new inintializers though.

@joshaber
Copy link
Member

joshaber commented Jul 9, 2013

Close! Mostly just a couple style notes.

@5sw
Copy link
Contributor Author

5sw commented Jul 9, 2013

OK, fixed those style issues, added the better error message and some unit tests for the error-returning initializers.

@joshaber
Copy link
Member

joshaber commented Jul 9, 2013

💥 🎆

Thanks!

joshaber added a commit that referenced this pull request Jul 9, 2013
@joshaber joshaber merged commit 96dfe9b into libgit2:master Jul 9, 2013
@dannygreg
Copy link
Contributor

phatblat pushed a commit to phatblat/objective-git that referenced this pull request Sep 13, 2014
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