-
Notifications
You must be signed in to change notification settings - Fork 413
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
Merge extension with extended type #343
Merge extension with extended type #343
Conversation
I went ahead and got this one ready, now that @jpsim big Obj-C push is done. It’s now based off of master instead of swift-2.0, and the specs are up to date. This is ready to merge if you’re happy with the output. As a reminder of issues from the older discussion:
|
end | ||
|
||
def protocol? | ||
kind =~ /^source\.lang\.swift\.decl\.protocol$/ |
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.
it seems nicer/cleaner to just do a string comparison if we're matching for an exact string anyway?
This is so so great. I'm still impressed at how concise you managed to keep this code. 👍 other than my very minor comments. Also, although this is something I haven't done yet, there are lots of areas now where Swift processing differs from Objective-C processing, and we should at some point go through the codepaths to branch off where needed to keep things efficient. |
That’s kind of you. I actually found it a little hard to read on re-review, so I did a very light refactoring that I think helps. @jpsim, I’ve made your very sensible fixes, so if CI likes it, we’re good to go. |
Looks like there are rubocop violations to address:
|
Yeah, weird — why didn’t those show up locally? Fixing… |
Not getting that rubocop failure locally. I notice that I have rubocop 0.26.1 locally, which is what’s in the Gemfile.lock, but Travis is downloading 0.34.2. ?!? Just pushed a commit that’s a shot in the dark at what I think rubocop is asking for. We’ll see if it passes. |
Rubocop is pinned at In any case, your shot in the dark passed! 🎉 I'll add a changelog entry for you in the spirit of getting this in. |
…type Merge extension with extended type
GTG |
Oh, right , changelog. Thanks. |
Continuing #289 based off of master instead of swift-2.0.