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

Refactoring, short-circuiting #4

Closed lorensr closed 8 years ago

lorensr commented 8 years ago

Not ready to merge, I haven't tested yet. Probably even has syntax errors 😜

Changes are more intelligible if you look at individual commits.

// Hold the launch screen
const handle = LaunchScreen.hold();

// Just release the splash screen
handle.release();

takes longer to read and understand than this:

const launchScreen = LaunchScreen.hold();

launchScreen.release();

a lot of readbility has to do with naming and structure (handle -> launchScreen). similarly, note how I extracted out pieces of code, or compound booleans, into functions with names that were descriptive enough that no comment was necessary.

lorensr commented 8 years ago

Did your test app have much in it, or just a call to configure that you manually changed during testing?

jamielob commented 8 years ago

This looks great @lorensr! I'll take a better look tomorrow. Test app was just a manual testing app.

lorensr commented 8 years ago

tests don't work, waiting on https://github.com/meteor/guide/issues/401

lorensr commented 8 years ago

actually probably a different problem: http://stackoverflow.com/questions/36855256/what-could-cause-an-onuse-package-to-not-be-defined-during-a-package-test

lorensr commented 8 years ago

tests working, would you like to add to them?

lorensr commented 8 years ago

production-ready packages have tests and changelogs 😉 😄 good thing we're past 1.3 and don't have to use tinytest! https://github.com/lorensr/login-links/blob/master/tests/client/connectionLogin.js

jamielob commented 8 years ago

You're a star! I'm just finishing up some contract work and merging all this is next on my list.

lorensr commented 8 years ago

Thanks! It was enough changes that I wouldn't publish until more tests are written or it's re-manually tested