therubymug / hitch

Git author attribution helper for pair programmers.
http://github.com/therubymug/hitch
MIT License
250 stars 32 forks source link

Explicitly convert inputted email address to string #18

Closed iamvery closed 10 years ago

iamvery commented 10 years ago

Using hitch with Ruby 2.0 produces weird values for the "group email". This is due to a serialization issue with highline which appears to trickle all the way down to Ruby's YAML parser psych (see the above referenced issue).

See https://gist.github.com/iamvery/7218658 for example "bad" serialization.

therubymug commented 10 years ago

@iamvery thanks! I wonder if simply bumping the version of highline fixes the issue?

iamvery commented 10 years ago

@therubymug that's an open and unresolved issue, so it would seem not :/. However I haven't independently verified that assumption. There may be a bit of work required to resolve the issue from that side, and it would seem no one is terribly motivated.

I figured this is a legitimate solution to our particular problem since we are in fact assuming that the result of user input is a strong. Thanks! :)

iamvery commented 10 years ago

Also, fwiw, I didn't run into these issues for other inputted values. That may be due to the lack of manipulation/validation for other inputs. I didn't dig too deep.

therubymug commented 10 years ago

@iamvery do you think you could reproduce the error in a test in Hitch? This would leave a regression test in Hitch, for posterity's sake! Thanks for narrowing it down though! I haven't had the time to dig into this. :-)

iamvery commented 10 years ago

@therubymug absolutely yes. I can and should have done this. It may take a little more time to get back around to it, but you've got it :dancer:

iamvery commented 10 years ago

With a little more research, I've learned some more about this. This is actually a problem that comes up with a more recent version of highline than is in the project's Gemfile.lock. I've got an updated patching coming with a spec, but it may be a little more than we want to include in such a PR. We can talk about it :smiley:

iamvery commented 10 years ago

@therubymug is there anything I can do to help you get this ready for acceptance? Thanks!

therubymug commented 10 years ago

@iamvery I somehow missed your update! :-/ I'll take a look-see but at first blush it looks good.

iamvery commented 10 years ago

No worries