Skip to content

Give GTTag Some Love #253

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 13 commits into from
Sep 10, 2013
Merged

Give GTTag Some Love #253

merged 13 commits into from
Sep 10, 2013

Conversation

PiersonBro
Copy link
Contributor

  • Converts -(NSString *)message, -(NSString *)name, -(GTObject *)target, and -(NSString *)targetType to properties.
  • Updates syntax for some of those properties.
  • Converts GTTagTest to GTTagSpec.

= added 3 commits September 6, 2013 12:49
… and -(NSString *)targetType to properties.
(The previous commit accidentally made those changes, this just updates the Project Settings)
@dannygreg
Copy link
Contributor

Hot.

While you're in here could you add some documentation to the properties?

@ghost ghost assigned dannygreg Sep 9, 2013
@@ -38,13 +38,13 @@

@property (nonatomic, readonly, strong) GTSignature *tagger;

// The underlying `git_object` as a `git_tag` object.
- (git_tag *)git_tag __attribute__((objc_returns_inner_pointer));
@property (nonatomic,strong) NSString *message;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm misreading the implementation, these don't seem to be writable (but the declarations say they are).

Copy link
Contributor

Choose a reason for hiding this comment

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

Good spot, these should be readonly.

Also a space between each attribute please, (nonatomic, readonly).

@PiersonBro
Copy link
Contributor Author

@@ -54,4 +59,9 @@
// Returns the found object or nil on error.
- (id)objectByPeelingTagError:(NSError **)error;

// The underlying `git_object` as a `git_tag` object.
- (git_tag *)git_tag __attribute__((objc_returns_inner_pointer));

Copy link
Contributor

Choose a reason for hiding this comment

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

All this whitespace is unnecessary.

@dannygreg
Copy link
Contributor

🎆

Just a few notes. Thanks for cleaning up this legacy crud!

@PiersonBro
Copy link
Contributor Author

🎯

}

- (NSString *)name {
return [NSString stringWithUTF8String:git_tag_name(self.git_tag)];
return @(git_tag_name(self.git_tag));
}

- (GTObject *)target {
git_object *t;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we fix the rogue spacing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't see it. :(

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it's a tabs vs spaces issue. Looks like this used to use spaces at some point.

@dannygreg
Copy link
Contributor

🍦

So close… just gotta tidy up some of the old spacing in here.

@PiersonBro
Copy link
Contributor Author

Let's hope this commit fixed it. Not sure though.

if(gitError < GIT_OK) return nil;
return [GTObject objectWithObj:(git_object *)t inRepository:self.repository];
if (gitError < GIT_OK) return nil;

Copy link
Contributor

Choose a reason for hiding this comment

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

No need to add this whitespace.

@dannygreg
Copy link
Contributor

We can run with it. Just need to tighten up one more bit of whitespace then this is good to :shipit:.

@PiersonBro
Copy link
Contributor Author

🚢

@dannygreg
Copy link
Contributor

💥🐫

dannygreg pushed a commit that referenced this pull request Sep 10, 2013
@dannygreg dannygreg merged commit 26973e2 into libgit2:master Sep 10, 2013
@PiersonBro PiersonBro deleted the Give-GTTag-Some-Love branch September 11, 2013 00:11
@PiersonBro
Copy link
Contributor Author

Yay! Thanks for working with me on this!

@dannygreg
Copy link
Contributor

No problem at all! Thanks for contributing!

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