to_lang: my first gem, looking for feedback

[Jimmy C.]

Greetings,

I've been working on a library that adds language translation methods to
strings using the Google Translate API. It's my first time writing a
gem, using RSpec, and documenting with YARD. I've gotten it to a point I
feel is worth sharing but I'd love any feedback anyone can provide on my
specs, the documentation, or the code in general. Feel free to be harsh
if it's bad, cause I'd like to learn and get better.

The source code is on GitHub: https://github.com/jimmycuadra/to_lang

Thanks in advance.

P.S. The rules say no posting advertisements - I hope this doesn't come
across as one. Apologies if this is against the rules somehow.

 

[Ryan Davis]

The source code is on GitHub: https://github.com/jimmycuadra/to_lang
=20
Thanks in advance.
=20
P.S. The rules say no posting advertisements - I hope this doesn't = come
across as one. Apologies if this is against the rules somehow.

That's different. This is more than allowed. Otherwise I'd be tarred and =
feathered by now. =

 

[Benoit Daloze]

Greetings,

I've been working on a library that adds language translation methods to
strings using the Google Translate API. It's my first time writing a
gem, using RSpec, and documenting with YARD. I've gotten it to a point I
feel is worth sharing but I'd love any feedback anyone can provide on my
specs, the documentation, or the code in general. Feel free to be harsh
if it's bad, cause I'd like to learn and get better.

The source code is on GitHub: https://github.com/jimmycuadra/to_lang

Thanks in advance.

P.S. The rules say no posting advertisements - I hope this doesn't come
across as one. Apologies if this is against the rules somehow.

You might be aware, but there is already a Google translate gem:
https://github.com/caius/gtranslate

It seems very well documented and the code is fine IMO.

For the API, I prefer "org_to_dst" to "to_dst_from_org", but that is personal.

A detail, but
You define String#{method_missing,respond_to?}, and might override
these methods if they were defined by another library.
You should probably copy the old method with alias, and call it
instead of super.

 

[Jimmy C.]

Benoit Daloze wrote in post #970986:

You might be aware, but there is already a Google translate gem:
https://github.com/caius/gtranslate

I actually had not seen that one. I was aware of the rtranslate gem, but
gtranslate seems more similar to mine. I did this mostly as a learning
exercise so it's all right with me that something similar already
exists. My gem also uses a newer version of the GT API.

It seems very well documented and the code is fine IMO.

For the API, I prefer "org_to_dst" to "to_dst_from_org", but that is
personal.

I thought about this, and you're right that to_dst_from_org reads
strangely. I will likely add dynamic methods for the reverse-ordered
version in the future so people can use whichever they prefer.

A detail, but
You define String#{method_missing,respond_to?}, and might override
these methods if they were defined by another library.
You should probably copy the old method with alias, and call it
instead of super.

Good advice. Added here:
https://github.com/jimmycuadra/to_lang/commit/c6966bb6394f79d4383fbf4e9b0861e4cb7df3af

Thanks very much for your input, Benoit!

 

[Jimmy C.]

Aaron Patterson wrote in post #970994:

When you use `File.expand_path`, you're *forcing* that file to be in a
place relative to this file. That may seem OK, but what you're really
saying is "I do not want ruby to consult the $LOAD_PATH when requiring".

Where this really becomes a hindrance is if someone (even yourself) want
to provide an alternate implementation of say "to_lang/connector", you
could just change the -I flags and provide the correct file. But with
the `expand_path` form, you've prevented any hope of doing that.

I notice you only changed a couple usages of `File.expand_path`, but
left others alone. Is it somehow correct to use them in these other
places (e.g. string_methods.rb and connector_spec.rb) or were you just
leaving it for me to clean up the rest? I'm not sure how else I'd do it
in the remaining cases, since the specs are not in the load path and
string_methods.rb is already inside "lib/to_lang" which makes me think
it'd end up attempting to load "lib/to_lang/to_lang/codemap.rb" instead
of "lib/to_lang/codemap.rb". I'm probably just not fully grasping how
the load path works.

Your other notes were very helpful and I have merged your commits into
my master. Thanks a bunch for taking the time to do this.

 

[Jimmy C.]

Jimmy C. wrote in post #971013:

I notice you only changed a couple usages of `File.expand_path`, but
left others alone. Is it somehow correct to use them in these other
places (e.g. string_methods.rb and connector_spec.rb) or were you just
leaving it for me to clean up the rest? I'm not sure how else I'd do it
in the remaining cases, since the specs are not in the load path and
string_methods.rb is already inside "lib/to_lang" which makes me think
it'd end up attempting to load "lib/to_lang/to_lang/codemap.rb" instead
of "lib/to_lang/codemap.rb". I'm probably just not fully grasping how
the load path works.

Disregard this. I did some reading on it and understand it better. I
removed all uses of `File.expand_path` from the project. Thanks again!

 

[Phillip Gawlowski]

That's different. This is more than allowed. Otherwise I'd be tarred and feathered by now.

Don't worry, you are on The List.

Anyway.

Announcing new releases of gems / Ruby programs / Ruby versions etc.
are at home in ruby-talk, too.

The rules are more about p3|\|1le enhancements, or unsolicited job offers.

--
Phillip Gawlowski

Though the folk I have met,
(Ah, how soon!) they forget
When I've moved on to some other place,
There may be one or two,
When I've played and passed through,
Who'll remember my song or my face.