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.

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

Fix gemfile to use local gems #221

Merged
merged 4 commits into from May 4, 2018

Conversation

Projects
None yet
6 participants
@seuros
Copy link
Member

commented Apr 24, 2018

No description provided.

@seuros seuros requested a review from apotonick Apr 24, 2018

@kressh

This comment has been minimized.

Copy link

commented Apr 25, 2018

Isn't it better to use bundle overrides? Like:

bundle config local.trailblazer-operation ../trailblazer-operation
bundle config local.trailblazer-macro ../trailblazer-macro
@seuros

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2018

No, you can't when you have a gem being fetched from a VCS.

You have to match the branches by adding branch to the gemfile.

Testing with the env variable and again without it is much easier than changing bundle config everytime. wdyt ?

@fernandes

This comment has been minimized.

Copy link
Member

commented Apr 27, 2018

@seuros suggestion: can add scripts/use_local_gems.sh that does the bundle config and scripts/use_github_gems.sh that undo the bundle config

just evaluate what works better, not the intention to bikeshed here heheh

ps: I'm +1 for a bundle config solution because it's related to developers machine, not a global config... anyway 😉

@apotonick

This comment has been minimized.

Copy link
Collaborator

commented Apr 27, 2018

I'm against bundle config... I've tried it a few times and found myself going back to messing around with the Gemfile because it's much more explicit. Maybe the main problem here is that we had to work a lot with "edge versions" of our gems, and we're now slowly at a point where things are stable. So, maybe this issue will solve itself?

@seuros

This comment has been minimized.

Copy link
Member Author

commented Apr 27, 2018

@fernandes it won't work. bundle config require to set the branch.
and when the local folder is checking another branch, it will fail.

Gemfile Outdated
gem "trailblazer-macro", github: "trailblazer/trailblazer-macro"
gem "trailblazer-macro-contract", github: "trailblazer/trailblazer-macro-contract"
gem "trailblazer-activity", github: "trailblazer/trailblazer-activity"
end

This comment has been minimized.

Copy link
@n-rodriguez

n-rodriguez May 1, 2018

IMHO it would better to use git: https://github.com/trailblazer/trailblazer-operation.git instead of github.
github uses the Git protocol to download sources which is not secured.

This comment has been minimized.

Copy link
@fernandes

fernandes May 1, 2018

Member
git_source(:github) do |repo_name|
  repo_name = "#{repo_name}/#{repo_name}" unless repo_name.include?("/")
  "https://github.com/#{repo_name}.git"
end

and there we fixed it! 👍

This comment has been minimized.

Copy link
@apotonick

apotonick May 14, 2018

Collaborator

Also, some clients etc. do not allow untrusted SSH connections, so HTTPS the way @n-rodriguez suggests is better.

@seuros seuros force-pushed the fixGemfile branch from be7c4f2 to 114dd05 May 3, 2018

@seuros seuros force-pushed the fixGemfile branch from 114dd05 to 25add53 May 3, 2018

Reform::Form.class_eval do
include Reform::Form::ActiveModel::Validations
# TODO: convert tests to non-rails.
require "reform"

This comment has been minimized.

Copy link
@saiqulhaq

saiqulhaq May 4, 2018

there are many trailing whitespace here

@seuros seuros merged commit 5ba1ac0 into master May 4, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@seuros seuros deleted the fixGemfile branch May 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.