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

make it rails independent #57

Closed

Conversation

ivanovaleksey
Copy link
Contributor

@ivanovaleksey ivanovaleksey commented Sep 4, 2016

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.

@@ -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__))

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.

Copy link
Contributor Author

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 😄

Copy link
Owner

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.

@@ -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 }

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.

@ivanovaleksey
Copy link
Contributor Author

I found very simple Sinatra application, remove some stuff and push it.
The application uses the gem from my fork.
It seems like it works fine.
I created db/seeds directory with dummy files.
The gem defines the tasks correctly so we can see them by running rake -T and execute them via rake.

@james2m
Copy link
Owner

james2m commented Sep 8, 2016

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; }

@ivanovaleksey
Copy link
Contributor Author

James but I didn't update tests code. All the tests now are the same.
So they are still Rails dependent.
I thought about changing them somehow but I have no idea how to remove Rails from them (I also not sure if it is really possible and should be done).
By the way what this css is about? ☺️

@james2m
Copy link
Owner

james2m commented Jan 15, 2017

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 ¯_(ツ)_/¯

@ivanovaleksey
Copy link
Contributor Author

Hi James, yes, I will try to complete this PR.

#60 introduces a new littile Rails dependency so rebase would be required.

@james2m
Copy link
Owner

james2m commented Feb 12, 2017

This is manually merged onto master.

@james2m james2m closed this Feb 12, 2017
@ivanovaleksey
Copy link
Contributor Author

Hi James, thank you!
I am sorry because I didn't have enough time to complete this 😞
But I definitely will test it once again with simple Sinatra app (which I would share).
What do you mean by "manually merged"? I don't see any changes on master branch related to this.
Am I missing something?

@james2m
Copy link
Owner

james2m commented Feb 12, 2017

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.

@ivanovaleksey
Copy link
Contributor Author

Now I see the commit. Is there something else I can help you with (besides from testing it with Sinatra app)?

@james2m
Copy link
Owner

james2m commented Feb 12, 2017

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.

@james2m
Copy link
Owner

james2m commented Feb 12, 2017

Back on topic, I'll leave this open until we have some tests in place for non rails apps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use with Sinatra or Padrino?
3 participants