search

akanshmurthy / railsbridge-testfest

The skeleton app.
MIT License
4 stars 2 forks source link

Close #5 #16

Closed siruguri closed 7 years ago

siruguri commented 7 years ago

It has a Bitly test that needs code to pass; it fixes up .gitignore a bit; and adds a User fixture.

siruguri commented 7 years ago

The bitly gem has a bug. At least the one by @philnash on GitHub. He's working on fixing it.

Typed on a phone

On Nov 3, 2016 12:54 AM, "Sanderfer" notifications@github.com wrote:

@schaui6 commented on this pull request.

In spec/controllers/posts_controller_spec.rb https://github.com/akanshmurthy/railsbridge-hackathon/pull/16:

@@ -89,5 +90,39 @@ end end

  • describe "#create" do

looks great! it does look like a lot, but it would be great to break down and walk through with students. Maybe include the bitly gem, to leave a hint for students? Should we merge the pull request?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/akanshmurthy/railsbridge-hackathon/pull/16, or mute the thread https://github.com/notifications/unsubscribe-auth/AACezSRqyl4iHAK3zRDhpsQnxsTzUl7Xks5q6ZMigaJpZM4Kn_0E .

philnash commented 7 years ago

Hello! Which bug is that @siruguri? Can I help?

siruguri commented 7 years ago

Oh. Didn't realize this wd show up on your mentions :)

In class Bitly, api_version = 3, should read self.api_version = 3, in the method where you set the use of v3.

Thanks !

Typed on a phone

On Nov 3, 2016 4:46 AM, "Phil Nash" notifications@github.com wrote:

Hello! Which bug is that @siruguri https://github.com/siruguri? Can I help?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/akanshmurthy/railsbridge-hackathon/pull/16#issuecomment-258121790, or mute the thread https://github.com/notifications/unsubscribe-auth/AACezTtFwKAkhk1qslFeeEjOFa08QS81ks5q6cmfgaJpZM4Kn_0E .

philnash commented 7 years ago

Oh man, that's a stupid bug. Let me sort that out. Thanks!

philnash commented 7 years ago

Ok, try Bitly version 1.0.2. Should sort this out.

siruguri commented 7 years ago

@philnash - thank you, sir! Much appreciated.

Others: the fix notwithstanding, may I suggest leaving the "bare metal" GET call in there, as a better didactic tool, than going the "there's a gem for that" route? Bitly's API is simple enough (esp for .shorten) that the code remains largely readable.

schaui6 commented 7 years ago

good point! i'm ok with leaving out the gem

philnash commented 7 years ago

Just since I've been part of this now and noticed it...

You added the bitly gem to the Gemfile and installed it, but you're not using it. You now have an extra dependency that's unnecessary given your application code.

You should also look out for net/http errors and rescue them when making your call to the Bitly API.