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

WIP add recipe for sinatra::reloader #70

Merged
merged 1 commit into from May 6, 2016

Conversation

ashleygwilliams
Copy link
Member

@zzak asked for a Rack::Reloader recipe in #44

this isn't that, but:

  1. it's worthwhile to have this alongside the shotgun recipe, since that doesn't work for Windows.
  2. what's the motivation behind a rack::reloader recipe, exactly?

this is a bit of a rewrite of http://www.sinatrarb.com/contrib/reloader.html which i think is a lil confusing (don't weave classic + modular app implementations, just present each individually, IMO)

i'm also going to write up a recipe for rerun. do we think that reloader, shotgun, and rerun should all be one article on reloaders? 3 makes a category, right, heh?

@zzak
Copy link
Member

zzak commented Sep 13, 2013

Sinatra::Reloader docs should go in contrib, see #56

@ashleygwilliams
Copy link
Member Author

sure, but imagine the use case where a windows user goes to sinatra recipes and wants a reloader. we don't have to include the article, but there should be a single reloader page then that lists shotgun, rerun, and at least references the contrib project; not doing that makes discovery of reloader super hard. thoughts?

@zzak
Copy link
Member

zzak commented Sep 16, 2013

@ashleygwilliams We actually had a few issues with reloader on windows, some of them might be resolved with the latest contrib release (after removing eventmachine as a dependency).

I'm not against a short reference mentioning the gem, by all means.

@zzak
Copy link
Member

zzak commented Apr 4, 2014

@ashleygwilliams ping agdubz

@kgrz
Copy link
Member

kgrz commented May 6, 2016

Ashley mentioned that I can merge these PRs if they are okay. Will merge this since this looks like a good starting point for reloaders.

@kgrz kgrz merged commit d740e2c into sinatra:master May 6, 2016
@zzak
Copy link
Member

zzak commented May 8, 2016

@kgrz In retrospect, I don't think this is enough to warrant a recipe.

If any of this information is missing from the sinatra-contrib documentation, that would be the appropriate place. IMO.

@kgrz
Copy link
Member

kgrz commented May 11, 2016

hmm, okay. Do you want me to revert this then?

@zzak
Copy link
Member

zzak commented May 11, 2016

@kgrz Yeah, as long as we plan to support reloader, we should do our best to give the first-class docs our love <3

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.

None yet

3 participants