Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add CLI - Integrate fakerbot 🤖 #1507

Merged
merged 17 commits into from
Mar 3, 2019
Merged

Add CLI - Integrate fakerbot 🤖 #1507

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e34a04b
feat(cli): Pull in fakerbot gem logic
akabiru Jan 4, 2019
552de6e
chore(cli): Extract module outside of `faker` dir
akabiru Jan 4, 2019
2582f5d
feat(cli): Add `faker` excecutable
akabiru Jan 4, 2019
38ec79c
feat(cli): Add renderer; reflector unit tests
akabiru Jan 4, 2019
c5e7224
feat(cli): Add commands tests
akabiru Jan 4, 2019
5a65094
feat(cli): Add integration tests
akabiru Jan 4, 2019
deb31a4
feat(cli): Add documentation 📖
akabiru Jan 5, 2019
96b2dd1
chore(cli): Extract constructor to base command
akabiru Jan 5, 2019
a4b2bc2
fix(cli): Amend faker version load path
akabiru Jan 5, 2019
9285086
fix(cli): Skip default deprecation warning output in the CLI
akabiru Jan 5, 2019
63a24f1
chore(cli): Amend var names
akabiru Jan 6, 2019
fd9d986
chore(reflectors): Distribute Reflector responsibilities
akabiru Jan 22, 2019
203ef41
Merge master
akabiru Feb 13, 2019
450dbaf
Merge branch 'master' into feature/pull-in-fakerbot
akabiru Feb 15, 2019
a800621
chore: Don't specify bunlder version
akabiru Feb 15, 2019
7ca6e5d
chore: Address review comments
akabiru Feb 17, 2019
491b63a
chore: Skip Gem::Deprecate warning output in CLI
akabiru Feb 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
chore: Don't specify bunlder version 
There's an erroneous 'null byte' error in the CI which could be caused
by the bundler version.

Further, As of Ruby 2.6.0, bundler is now built into Ruby, so no need to
install it ourselves
  • Loading branch information
@akabiru
akabiru committed Feb 15, 2019
commit a80062191d02d1903ad24f06b423a011de82a903
3 changes: 1 addition & 2 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ PLATFORMS
ruby

DEPENDENCIES
bundler (= 1.16.4)
faker!
minitest (= 5.11.3)
pry (= 0.12.2)
Expand All @@ -85,4 +84,4 @@ DEPENDENCIES
timecop (= 0.9.1)

BUNDLED WITH
1.17.1
1.17.3
1 change: 0 additions & 1 deletion faker.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ Gem::Specification.new do |spec|
spec.add_dependency('tty-screen', '~> 0.6.5')
spec.add_dependency('tty-tree', '~> 0.2.0')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding these dependencies is a pretty big deal. Are we comfortable with them? I saw the terminal response. It's pretty cool.

Thoughts? @stympy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a valid concern, from my POV, these libraries go a long way in making the CLI interface easier to work with. 🙂 The alternative would be to write it all ourselves which I'm not sure about

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I find all these dependencies to be a big deal. I opened an issue about it (#1642) and plan to stay on 1.9.3 for the time being.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@myronmarston your concern is valid.

@akabiru would you agree to remove these dependencies and just show a formatted list of searched results?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey folks, apologies for the very delayed response (on vacation). I think the move to a separate gem is the sensible approach. And allows users to opt-in into using the gem.

It did start as a separate gem , no strong objections on this one. 🙂

@vbrazo I'm happy to assist with the extraction &/migration as well.


spec.add_development_dependency('bundler', '1.16.4')
spec.add_development_dependency('minitest', '5.11.3')
spec.add_development_dependency('pry', '0.12.2')
spec.add_development_dependency('rake', '12.3.1')
Expand Down