jamielob / reloader

More control over hot code push reloading for your production apps. Designed to replace mdg:reload-on-resume and provide a more production-ready approach.
28 stars 22 forks source link

Version 2 #5

Closed jamielob closed 7 years ago

jamielob commented 8 years ago

I need to unify the language between the words "Refresh" and "Reload" as they are both used interchangeably at the moment.

I think that Reload is the better term so I propose that I change any references to refresh to reload.

cc: @lorensr for comments.

lorensr commented 8 years ago

👍 can also support refresh config as an alias to maintain backward compat

jamielob commented 8 years ago

Do you think we really need to worry about that when no one's using it from Atmosphere yet? We could just bump to 2.0.0 and record in Changelog. My fault for being hasty to publish in the first place.

lorensr commented 8 years ago

Starting w/ version <1 is nice b/c you can make incompatible changes: http://semver.org/#spec-item-5

Yeah 2.0 is good too. If we can change API, what do you think about:

jamielob commented 8 years ago

So Reload.updateAvailable()? Yep, that looks better to me.

Definitely agree with starting with a more conservative default. I think starting with the most conservative makes sense. Conservative is a relative term though, because if you want your app to be up to date no matter what, then it's actually the least conservative!

lorensr commented 8 years ago

Mind adding me on this repo? 😁

So which are you thinking? I'm actually feeling like

{
  check: 'firstStart'
  checkIdleCutoff: N/A
  checkTimer: 3000
  reload: 'startAndResume'
  reloadIdleCutoff: 1 hour? dunno. just don't want it restarting when the user is multitasking, and about to come back to the app soon.
  launchScreenDelay: 0
}
jamielob commented 8 years ago

With pleasure. Let me think on that. I think we may also need to be able to configure it to allow you to say if the app is restarted in ANY way within X seconds, then don't don't consider it a start. So if your app was closed because of memory when multi-tasking and therefore didn't fire the resume event, you wouldn't get stuck with the reload when you didn't expect it. Basically, I'm not sure if the resume event is reliable enough.

lorensr commented 8 years ago

is pause reliable enough, or is it X seconds since the last start?

On Tue, Apr 26, 2016 at 6:21 PM, Jamie Loberman notifications@github.com wrote:

With pleasure. Let me think on that. I think we may also need to be able to configure it to allow you to say if the app is restarted in ANY way within X seconds, then don't don't consider it a start. So if your app was closed because of memory when multi-tasking and therefore didn't fire the resume event, you wouldn't get stuck with the reload when you didn't expect it. Basically, I'm not sure if the resume event is reliable enough.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/jamielob/reloader/issues/5#issuecomment-214905406

jamielob commented 8 years ago

I think the pause event is reliable - but I believe that it doesn't trigger resume if the app is auto-closed by memory management.

Partially relevant: https://github.com/jamielob/reloader/issues/3

lorensr commented 8 years ago

Oh, also maybe add InMilliseconds to time config, like Meteor does w/ Accounts.loginExpirationInDays.

On Tue, Apr 26, 2016 at 6:26 PM, Loren Sands-Ramshaw lorensr@gmail.com wrote:

is pause reliable enough, or is it X seconds since the last start?

On Tue, Apr 26, 2016 at 6:21 PM, Jamie Loberman notifications@github.com wrote:

With pleasure. Let me think on that. I think we may also need to be able to configure it to allow you to say if the app is restarted in ANY way within X seconds, then don't don't consider it a start. So if your app was closed because of memory when multi-tasking and therefore didn't fire the resume event, you wouldn't get stuck with the reload when you didn't expect it. Basically, I'm not sure if the resume event is reliable enough.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/jamielob/reloader/issues/5#issuecomment-214905406

jamielob commented 8 years ago

Reloader.reloadIdleCutoffInMillisecondsNotDaysMakeSureYouGetItRight;

lorensr commented 8 years ago

I don't know, that does have added clarity, but I'm worried it might be overly verbose.

On Tue, Apr 26, 2016 at 6:35 PM, Jamie Loberman notifications@github.com wrote:

Reloader.reloadIdleCutoffInMillisecondsNotDaysMakeSureYouGetItRight;

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/jamielob/reloader/issues/5#issuecomment-214908145

lorensr commented 8 years ago

Let's do the work on v2 branch so master readme matches atmosphere

jamielob commented 8 years ago

Yep!

jamielob commented 7 years ago

@lorensr - closing the v2 issues since it looks like we've both moved on to React Native and probably won't improve this package.