Skip to content

Commit 6df9faf

Browse files
committed
Fixes per @tiennou feedback
Some stylistic and other fixes based on @tiennou feedback on the pull request.
1 parent fe13986 commit 6df9faf

File tree

6 files changed

+100
-57
lines changed

6 files changed

+100
-57
lines changed

ObjectiveGit/GTNote.h

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,31 @@ NS_ASSUME_NONNULL_BEGIN
5151
@property (nonatomic, readonly, strong) GTObject *target;
5252

5353
/// The underlying `git_note` object.
54-
- (git_note * _Nullable)git_note __attribute__((objc_returns_inner_pointer));
54+
- (git_note *)git_note __attribute__((objc_returns_inner_pointer));
5555

56-
/// These initializers may fail if there's no note attached to the provided oid.
57-
- (nullable instancetype)initWithTargetOID:(GTOID*)oid repository:(GTRepository*)repository ref:(nullable NSString*)ref;
58-
- (nullable instancetype)initWithTargetGitOID:(git_oid*)oid repository:(git_repository *)repository ref:(const char* _Nullable)ref;
56+
/// Create a note with target OID in the given repository.
57+
///
58+
/// oid - OID of the target to attach to
59+
/// repository - Repository containing the target OID refers to
60+
/// referenceName - Name for the notes reference in the repo, or nil for default ("refs/notes/commits")
61+
/// error - Will be filled with a NSError object in case of error.
62+
/// May be NULL.
63+
///
64+
/// Returns initialized GTNote instance or nil on failure (error will be populated, if passed).
65+
- (nullable instancetype)initWithTargetOID:(GTOID *)oid repository:(GTRepository *)repository referenceName:(nullable NSString *)referenceName error:(NSError **)error;
66+
67+
/// Create a note with target libgit2 oid in the given repository.
68+
///
69+
/// oid - git_oid of the target to attach to
70+
/// repository - Repository containing the target OID refers to
71+
/// referenceName - Name for the notes reference in the repo, or NULL for default ("refs/notes/commits")
72+
/// error - Will be filled with a git error code in case of error.
73+
/// May be NULL.
74+
///
75+
/// Returns initialized GTNote instance or nil on failure (error will be populated, if passed).
76+
- (nullable instancetype)initWithTargetGitOID:(git_oid *)oid repository:(git_repository *)repository referenceName:(const char * _Nullable)referenceName error:(int * _Nullable)error;
77+
78+
- (instancetype)init NS_UNAVAILABLE;
5979

6080
@end
6181

ObjectiveGit/GTNote.m

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,19 +57,36 @@ - (GTOID*)targetOID {
5757
return [GTOID oidWithGitOid:git_note_id(self.git_note)];
5858
}
5959

60-
- (instancetype)initWithTargetOID:(GTOID*)oid repository:(GTRepository*)repository ref:(NSString*)ref {
61-
return [self initWithTargetGitOID:(git_oid *)oid.git_oid repository:repository.git_repository ref:ref.UTF8String];
60+
- (instancetype)initWithTargetOID:(GTOID *)oid repository:(GTRepository *)repository referenceName:(NSString *)referenceName error:(NSError **)error {
61+
int err = GIT_OK;
62+
63+
id object = [self initWithTargetGitOID:(git_oid *)oid.git_oid repository:repository.git_repository referenceName:referenceName.UTF8String error:&err];
64+
65+
if (err != GIT_OK && error != NULL) {
66+
*error = [NSError git_errorFor:err description:@"Failed to create a note."];
67+
}
68+
69+
return object;
6270
}
6371

64-
- (instancetype)initWithTargetGitOID:(git_oid*)oid repository:(git_repository *)repository ref:(const char*)ref {
65-
if (self = [super init]) {
66-
int gitErr = git_note_read(&_note, repository, ref, oid);
72+
- (instancetype)initWithTargetGitOID:(git_oid *)oid repository:(git_repository *)repository referenceName:(const char *)referenceName error:(int *)error {
73+
self = [super init];
74+
if (self == nil) return nil;
75+
76+
int gitErr = git_note_read(&_note, repository, referenceName, oid);
77+
78+
if (gitErr != GIT_OK) {
79+
if (error != NULL) *error = gitErr;
6780

68-
if (gitErr != GIT_OK)
69-
return nil; // Cannot read the note, means it either doesn't exists for this object, this object is not found, or whatever else.
81+
return nil;
7082
}
7183

7284
return self;
7385
}
7486

87+
- (instancetype)init {
88+
NSAssert(NO, @"Call to an unavailable initializer.");
89+
return nil;
90+
}
91+
7592
@end

ObjectiveGit/GTRepository+RemoteOperations.m

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,8 @@ - (BOOL)pushNotes:(NSString *)noteRef toRemote:(GTRemote *)remote withOptions:(N
217217
}
218218

219219
noteRef = [NSString stringWithUTF8String:default_ref_name.ptr];
220+
221+
git_buf_free(&default_ref_name);
220222
}
221223

222224
return [self pushRefspecs:@[[NSString stringWithFormat:@"%@:%@", noteRef, noteRef]] toRemote:remote withOptions:options error:error progress:progressBlock];

ObjectiveGit/GTRepository.h

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -607,45 +607,45 @@ typedef NS_ENUM(NSInteger, GTRepositoryStateType) {
607607

608608
/// Creates a new note in this repo (using a default notes reference, e.g. "refs/notes/commits")
609609
///
610-
/// note - Note text.
611-
/// theTarget - Object (usually a commit) to which this note attaches to.
612-
/// This object must belong to this repository.
613-
/// ref - Name for the notes reference in the repo, or nil for default ("refs/notes/commits")
614-
/// author - Signature of the author for this note, and
615-
/// of the note creation time
616-
/// committer - Signature of the committer for this note.
617-
/// overwrite - If set to YES, the note will be overwritten if it already exists.
618-
/// error - Will be filled with a NSError object in case of error.
619-
/// May be NULL.
610+
/// note - Note text.
611+
/// theTarget - Object (usually a commit) to which this note refers to.
612+
/// This object must belong to this repository.
613+
/// referenceName - Name for the notes reference in the repo, or nil for default ("refs/notes/commits")
614+
/// author - Signature of the author for this note, and
615+
/// of the note creation time
616+
/// committer - Signature of the committer for this note.
617+
/// overwrite - If set to YES, the note will be overwritten if it already exists.
618+
/// error - Will be filled with a NSError object in case of error.
619+
/// May be NULL.
620620
///
621621
/// Returns the newly created note or nil on error.
622-
- (nullable GTNote *)createNote:(NSString *)note target:(GTObject *)theTarget ref:(nullable NSString *)ref author:(GTSignature *)author committer:(GTSignature *)committer overwriteIfExists:(BOOL)overwrite error:(NSError **)error;
622+
- (nullable GTNote *)createNote:(NSString *)note target:(GTObject *)theTarget referenceName:(nullable NSString *)referenceName author:(GTSignature *)author committer:(GTSignature *)committer overwriteIfExists:(BOOL)overwrite error:(NSError **)error;
623623

624624
/// Removes a note attached to object in this repo (using a default notes reference, e.g. "refs/notes/commits")
625625
///
626-
/// parentObject - Object (usually a commit) to which the note to be removed is attached to.
627-
/// This object must belong to this repository.
628-
/// ref - Name for the notes reference in the repo, or nil for default ("refs/notes/commits")
629-
/// author - Signature of the author for this note removal, and
630-
/// of the note removal time
631-
/// committer - Signature of the committer for this note removal.
632-
/// error - Will be filled with a NSError object in case of error.
633-
/// May be NULL.
626+
/// parentObject - Object (usually a commit) to which the note to be removed is attached to.
627+
/// This object must belong to this repository.
628+
/// referenceName - Name for the notes reference in the repo, or nil for default ("refs/notes/commits")
629+
/// author - Signature of the author for this note removal, and
630+
/// of the note removal time
631+
/// committer - Signature of the committer for this note removal.
632+
/// error - Will be filled with a NSError object in case of error.
633+
/// May be NULL.
634634
///
635635
/// Returns the YES on success and NO on error.
636-
- (BOOL)removeNoteFromObject:(GTObject *)parentObject ref:(nullable NSString *)ref author:(GTSignature *)author committer:(GTSignature *)committer error:(NSError **)error;
636+
- (BOOL)removeNoteFromObject:(GTObject *)parentObject referenceName:(nullable NSString *)referenceName author:(GTSignature *)author committer:(GTSignature *)committer error:(NSError **)error;
637637

638638
/// Enumerates through all stored notes in this repo (using a default notes reference, e.g. "refs/notes/commits")
639639
///
640-
/// error - Will be filled with a NSError object in case of error.
641-
/// May be null.
642-
/// ref - Name for the notes reference in the repo, or nil for default ("refs/notes/commits")
643-
/// block - A block to be called on each encountered note object. The block accepts
644-
/// a reference to `note`, an `object` that is annotated with the note.
645-
/// If the block sets `stop` to YES, the iterator is finished.
640+
/// referenceName - Name for the notes reference in the repo, or nil for default ("refs/notes/commits")
641+
/// error - Will be filled with a NSError object in case of error.
642+
/// May be NULL.
643+
/// block - A block to be called on each encountered note object. The block accepts
644+
/// a reference to `note`, an `object` that is annotated with the note.
645+
/// If the block sets `stop` to YES, the iterator is finished.
646646
///
647647
/// Returns YES on overall success or NO on error of any kind.
648-
- (BOOL)enumerateNotesWithError:(NSError **)error ref:(nullable NSString *)ref usingBlock:(void (^)(GTNote *note, GTObject *object, BOOL *stop))block;
648+
- (BOOL)enumerateNotesWithReferenceName:(nullable NSString *)referenceName error:(NSError **)error usingBlock:(void (^)(GTNote *note, GTObject *object, BOOL *stop))block;
649649

650650
@end
651651

ObjectiveGit/GTRepository.m

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -922,21 +922,21 @@ - (BOOL)cleanupStateWithError:(NSError * _Nullable __autoreleasing *)error {
922922

923923
#pragma mark Notes
924924

925-
- (GTNote *)createNote:(NSString *)note target:(GTObject *)theTarget ref:(NSString *)ref author:(GTSignature *)author committer:(GTSignature *)committer overwriteIfExists:(BOOL)overwrite error:(NSError **)error {
925+
- (GTNote *)createNote:(NSString *)note target:(GTObject *)theTarget referenceName:(NSString *)referenceName author:(GTSignature *)author committer:(GTSignature *)committer overwriteIfExists:(BOOL)overwrite error:(NSError **)error {
926926
git_oid oid;
927927

928-
int gitError = git_note_create(&oid, self.git_repository, ref.UTF8String, author.git_signature, committer.git_signature, theTarget.OID.git_oid, [note UTF8String], overwrite ? 1 : 0);
928+
int gitError = git_note_create(&oid, self.git_repository, referenceName.UTF8String, author.git_signature, committer.git_signature, theTarget.OID.git_oid, [note UTF8String], overwrite ? 1 : 0);
929929
if (gitError != GIT_OK) {
930930
if (error != NULL) *error = [NSError git_errorFor:gitError description:@"Failed to create a note in repository"];
931931

932932
return nil;
933933
}
934934

935-
return [[GTNote alloc] initWithTargetOID:theTarget.OID repository:self ref:ref];
935+
return [[GTNote alloc] initWithTargetOID:theTarget.OID repository:self referenceName:referenceName error:error];
936936
}
937937

938-
- (BOOL)removeNoteFromObject:(GTObject *)parentObject ref:(NSString *)ref author:(GTSignature *)author committer:(GTSignature *)committer error:(NSError **)error {
939-
int gitError = git_note_remove(self.git_repository, ref.UTF8String, author.git_signature, committer.git_signature, parentObject.OID.git_oid);
938+
- (BOOL)removeNoteFromObject:(GTObject *)parentObject referenceName:(NSString *)referenceName author:(GTSignature *)author committer:(GTSignature *)committer error:(NSError **)error {
939+
int gitError = git_note_remove(self.git_repository, referenceName.UTF8String, author.git_signature, committer.git_signature, parentObject.OID.git_oid);
940940
if (gitError != GIT_OK) {
941941
if(error != NULL) *error = [NSError git_errorFor:gitError description:@"Failed to delete note from %@", parentObject];
942942
return NO;
@@ -945,11 +945,11 @@ - (BOOL)removeNoteFromObject:(GTObject *)parentObject ref:(NSString *)ref author
945945
return YES;
946946
}
947947

948-
- (BOOL)enumerateNotesWithError:(NSError **)error ref:(NSString *)ref usingBlock:(void (^)(GTNote *note, GTObject *object, BOOL *stop))block
948+
- (BOOL)enumerateNotesWithReferenceName:(NSString *)referenceName error:(NSError **)error usingBlock:(void (^)(GTNote *note, GTObject *object, BOOL *stop))block
949949
{
950-
git_note_iterator* iter = NULL;
950+
git_note_iterator *iter = NULL;
951951

952-
int gitError = git_note_iterator_new(&iter, self.git_repository, ref.UTF8String);
952+
int gitError = git_note_iterator_new(&iter, self.git_repository, referenceName.UTF8String);
953953

954954
if (gitError != GIT_OK)
955955
{
@@ -960,22 +960,21 @@ - (BOOL)enumerateNotesWithError:(NSError **)error ref:(NSString *)ref usingBlock
960960
git_oid note_id;
961961
git_oid object_id;
962962
BOOL success = YES;
963+
int iterError = GIT_OK;
963964

964-
while (git_note_next(&note_id, &object_id, iter) == GIT_OK) {
965+
while ((iterError = git_note_next(&note_id, &object_id, iter)) == GIT_OK) {
965966
NSError* lookupErr = nil;
966967

967-
GTNote* note = [[GTNote alloc] initWithTargetGitOID:&object_id repository:self.git_repository ref:ref.UTF8String];
968+
GTNote* note = [[GTNote alloc] initWithTargetGitOID:&object_id repository:self.git_repository referenceName:referenceName.UTF8String error:&gitError];
968969
if (note == nil) {
969-
if (error != NULL)
970-
*error = [NSError git_errorFor:GIT_ENOTFOUND description:@"Cannot create note"];
970+
if (error != NULL) *error = [NSError git_errorFor:gitError description:@"Cannot find a note"];
971971
success = NO;
972972
break;
973973
}
974974

975975
GTObject* obj = [self lookUpObjectByGitOid:&object_id error:&lookupErr];
976976
if (obj == nil && lookupErr != nil) {
977-
if (error != NULL)
978-
*error = lookupErr;
977+
if (error != NULL) *error = lookupErr;
979978
success = NO;
980979
break;
981980
}
@@ -989,6 +988,11 @@ - (BOOL)enumerateNotesWithError:(NSError **)error ref:(NSString *)ref usingBlock
989988

990989
git_note_iterator_free(iter);
991990

991+
if (iterError != GIT_OK && iterError != GIT_ITEROVER) {
992+
if (error != NULL)
993+
*error = [NSError git_errorFor:iterError description:@"Iterator error"];
994+
}
995+
992996
return success;
993997
}
994998

ObjectiveGitTests/GTNoteSpec.m

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@
4242

4343
NSError* err = nil;
4444

45-
GTNote *note = [repository createNote:@"Note text" target:initialCommit ref:nil author:sig committer:sig overwriteIfExists:YES error:&err];
45+
GTNote *note = [repository createNote:@"Note text" target:initialCommit referenceName:nil author:sig committer:sig overwriteIfExists:YES error:&err];
4646
expect(note).notTo(beNil());
4747
expect(err).to(beNil());
4848

49-
[repository enumerateNotesWithError:&err ref:nil usingBlock:^(GTNote *note, GTObject *object, BOOL *stop) {
49+
[repository enumerateNotesWithReferenceName:nil error:&err usingBlock:^(GTNote *note, GTObject *object, BOOL *stop) {
5050
expect(note).notTo(beNil());
5151
expect(object).notTo(beNil());
5252

@@ -62,17 +62,17 @@
6262

6363
NSError* err = nil;
6464

65-
GTNote *note = [repository createNote:@"Note text" target:initialCommit ref:nil author:sig committer:sig overwriteIfExists:YES error:&err];
65+
GTNote *note = [repository createNote:@"Note text" target:initialCommit referenceName:nil author:sig committer:sig overwriteIfExists:YES error:&err];
6666
expect(note).notTo(beNil());
6767
expect(err).to(beNil());
6868

69-
BOOL res = [repository removeNoteFromObject:initialCommit ref:nil author:sig committer:sig error:&err];
69+
BOOL res = [repository removeNoteFromObject:initialCommit referenceName:nil author:sig committer:sig error:&err];
7070
expect(@(res)).to(beTrue());
7171
expect(err).to(beNil());
7272

7373
NSMutableArray* notes = [NSMutableArray arrayWithCapacity:0];
7474

75-
[repository enumerateNotesWithError:&err ref:nil usingBlock:^(GTNote *note, GTObject *object, BOOL *stop) {
75+
[repository enumerateNotesWithReferenceName:nil error:&err usingBlock:^(GTNote *note, GTObject *object, BOOL *stop) {
7676
[notes addObject:note];
7777
}];
7878

0 commit comments

Comments
 (0)