-
Notifications
You must be signed in to change notification settings - Fork 92
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
make it rails independent #57
Conversation
@@ -9,7 +9,7 @@ | |||
|
|||
Rails.backtrace_cleaner.remove_silencers! | |||
|
|||
Seedbank.seeds_root = File.expand_path('dummy/db/seeds', __FILE__) | |||
Seedbank.application_root = Pathname.new(File.expand_path('../dummy', __FILE__)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I should prefer single quotes 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely prefer single quotes unless interpolation, it's a good signpost to look closer when there are double quotes.
b40601e
to
322c013
Compare
@@ -19,8 +23,8 @@ def matcher | |||
end | |||
|
|||
def self.load_tasks | |||
Dir[File.expand_path("tasks/*.rake", File.dirname(__FILE__))].each { |ext| load ext } | |||
Dir[File.expand_path('../tasks/*.rake', __FILE__)].each { |ext| load ext } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
I found very simple Sinatra application, remove some stuff and push it. |
Cool. Id been thinking how to run the tests in different environments so this could be helpful. a, pre, code, a:link, body { word-wrap: break-word !important; } |
James but I didn't update tests code. All the tests now are the same. |
Hey @ivanovaleksey sorry for being quiet, had a lot on in work and home life.. I was thinking about how to test with & without rails. Maybe you could add your mini Sinatra app under the test folder as the rails one is, gives me something to push around. Ha! Not sure where the css came from, I was traveling when that was sent so maybe something left over from an email reply ¯_(ツ)_/¯ |
Hi James, yes, I will try to complete this PR. #60 introduces a new littile Rails dependency so rebase would be required. |
This is manually merged onto master. |
Hi James, thank you! |
Hey @ivanovaleksey thanks for the work, you pushed things forward when I wasn't doing so! You should see your changes now on master with a few tweaks from the merge. Unfortunately it's late here and Travis is complaining so I'll have to do a little more to fix that, but it's otherwise looking good. |
Now I see the commit. Is there something else I can help you with (besides from testing it with Sinatra app)? |
If you fancy having a crack at #43 it's easy to re-create but I've not had time to add duplicate checking. I'd like it to bomb out with a polite error when a 2nd seed of the same name is defined. In the meantime I'm going to look at multi environment tests as I should cert against rails 5. |
Back on topic, I'll leave this open until we have some tests in place for non rails apps. |
Hope it will fix #53
It still works within my Rails 4 app, but I haven't tested it yet with Sinatra or Padrino.
So I think it is just an initial version.