[Issue #47] CLI naming clash when run under Bundler

[zph]

Fixes a load_path naming clash [#47] when bin/approvals is run under Bundler.

Looks like it's picking up a different CLI lib from Thor instead of the
Approvals::CLI.

Verified by renaming cli.rb to something else & updating the require
statement to avoid naming clash.

[zph]

Feel free to choose a different name for the file, but expanding it out to commandline.rb worked in my development env.

Also, 👍 for this gem. Saved me a fair bit of effort w/ a hairy logic-filled Rails view refactor.

[markijbema]

Thanks for noticing :)

This PR seems to shift the problem though. If someone has a commandline gem, this will break, right? I think the proper way would be to require 'approvals/cli' or 'approvals/commandline'. Since that's scoped within approvals it won't collide with anything (except perhaps hypothetical approval-plugins)

[markijbema]

Heh, cool. You discovered this gem to solve almost exactly the same usecase as I had to solve when I discovered it :)

[zph]

@markijbema This is true (re shifting problem). In retrospect, it was a this is 1am, this works rather than going through git history to see why it changed between v0.0.12 and v0.0.13.

I'll submit an updated PR :)

[zph]

2b0527a by @kytrinyx is where this was changed (ie mv lib/approvals/utilities/cli.rb lib/cli.rb).

@kytrinyx, what do you think of keeping cli.rb in a scoped location? It will help when approvals is operated under Bundler.

[kytrinyx]

Yeah, we should probably namespace it, but I want it to not live underneath the lib/approvals directory. Here's my thinking:

Everything under lib/approvals is the domain/business logic. For any UI (for example we might add a tiny sinatra app that lets you look at diffs in a browser), each UI would be a top-level file+dir within lib/, so maybe:

.
└── lib
    ├── app
    │   └── views-or-something.rb
    ├── app.rb
    ├── approvals
    │   ├── logic.rb
    │   └── stuff.rb
    ├── cli
    │   └── cli-specific-logic.rb
    └── cli.rb

[kytrinyx]

Thanks for fixing this! ❤️

[kytrinyx]

I've released v0.0.14

[zph]

😻 thanks for the super helpful gem!

[markijbema]

But isn't everything in lib/ 'global' over all gems? I was under the assumption that after loading a gem, everything there is requir-able. Therefore, lib/approvals is the only place where you can put files without conflicting with other gems. All this to the best of my understanding of ruby gems, please correct me if I'm wrong.

[kytrinyx]

It hadn't occurred to me, but when thinking about it, you're probably right.

[zph]

Based on what I saw w/ Bundler, it looks like anything in 'lib/*' is in the global path namespace.

So to make sure to namespace reliably, it ought to be under lib/approvals/...

I'll submit 2nd PR that puts it in lib/approvals/cli/ for convenience. It can be part of the next tag release 👍.

[kytrinyx]

The cli/cli.rb isn't very intuitive to me. In Ruby I expect to see this:

/something.rb
/something/interesting.rb
/something/unusual.rb

Where there's a top-level .rb file, and if there are related files, then they are in a sub-directory.

How about just lib/approvals/cli.rb?