asciidoctor / docbookrx

(An early version of) a DocBook to AsciiDoc converter written in Ruby.
MIT License
22 stars 49 forks source link

Avoid modified .bundle/config #47

Closed junaruga closed 8 years ago

junaruga commented 8 years ago

I think we should not need .bundle/config file to git repository. Because it is modified file, depending on user's environment. And its situation is not good for the development. For example, it prevents user to do "git rebase".

Before modification

$bundle install --path vendor/bundle

$ git status
...
  modified:   .bundle/config 
...

I can understand why you added the file, because of compiling nokogiri with system library. And we can set a environment variable to get same result.

After modification

$ bundle install --path vendor/bundle
Fetching gem metadata from https://rubygems.org/
Fetching version metadata from https://rubygems.org/
Resolving dependencies...
Installing rake 10.4.2
Installing diff-lcs 1.2.5
Installing mini_portile2 2.0.0
Installing rspec-support 3.4.1
Using bundler 1.12.4
Installing nokogiri 1.6.7.2 with native extensions <--Check this.
Installing rspec-core 3.4.4
Installing rspec-expectations 3.4.0
Installing rspec-mocks 3.4.1
Using docbookrx 1.0.0 from source at `.`
Installing rspec 3.4.0
Bundle complete! 3 Gemfile dependencies, 11 gems now installed.
Bundled gems are installed into ./vendor/bundle.

$ git status
On branch feature/modified-bundle-config
Your branch is up-to-date with 'origin/feature/modified-bundle-config'.
Untracked files:
  (use "git add <file>..." to include in what will be committed)

    vendor/

nothing added to commit but untracked files present (use "git add" to track)
mrietveld commented 8 years ago

@mojavelinux This is more your thing. Any objections?

mojavelinux commented 8 years ago

I agree, but I would like to see an update to the README. I sent an update to this PR.

I definitely want to encourage devs to install gems into the project because I'm growing weary of explaining to people how to isolate gems from the system. The instructions in my update to the README now make this clear. Once that is merged into this PR, then I'll be ready to move forward with this PR.

mojavelinux commented 8 years ago

Note that I removed the env variable from Rakefile. I don't think that's needed.

junaruga commented 8 years ago

@mojavelinux Thanks! I merged its update!

mojavelinux commented 8 years ago

Thanks! Let's proceed!

junaruga commented 8 years ago

@mojavelinux this pull-request has 3 commits. Do you like me to squash these 3 commits to 1 commit?

junaruga commented 8 years ago

I might find one mistake in the document.

Before $ bundle --path=.bundle/rubygems

After $ bundle install --path=.bundle/rubygems

junaruga commented 8 years ago

Ah $ bundle --path=.bundle/rubygems was actually same with $ bundle install --path=.bundle/rubygems.

mrietveld commented 8 years ago

@junaruga Please do -- the commits all relate to the same thing, so that would be better.

Thanks!

mojavelinux commented 8 years ago

Exactly, "install" is the default command.

mojavelinux commented 8 years ago

No need to squash. @mrietveld can do that using the Merge button on GitHub. Makes life simpler :)

mojavelinux commented 8 years ago

@mrietveld, I find that using the Merge (+ squash) button produces better results because it maintains the authors accurately.

mrietveld commented 8 years ago

@mojavelinux Ohh.. of course! Thanks, Dan.

junaruga commented 8 years ago

"the Merge (+ squash) button"?! Cool!

junaruga commented 8 years ago

It looks not merged to master branch yet. Is there something I have to do?