facebookarchive / instant-articles-builder

Instant Articles Rules Editor
https://facebook.github.io/instant-articles-builder
Other
125 stars 67 forks source link

Add Flow to pre-commit hook #16

Closed timjacobi closed 7 years ago

timjacobi commented 7 years ago

This will run Flow before committing and will stop the commit if Flow returns errors.

pestevez commented 7 years ago

Hey, @timjacobi - please see my comment in https://github.com/facebook/facebook-instant-articles-rules-editor/pull/15#issuecomment-346335897. I am not sure if we should block the commit because of flow and/or test issues. I'd love to know your thoughts.

timjacobi commented 7 years ago

Makes sense. What do you think about a pre push hook then?

pestevez commented 7 years ago

That takes care of allowing people to commit, but still blocks the pushing to a remote branch (perhaps as a way to ensure you have a backup of your work.) I think it ultimately depends on what we are considering a testable state. I feel the PR signals that state, but I don't have strong feelings against the pre-push approach, and we can always revisit if it becomes an issue for us or the community.