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.

Sign up for GitHub

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

Jump to bottom

Rework files for asset checking #258

Merged
merged 2 commits into from
Feb 7, 2016
Merged

Rework files for asset checking #258

merged 2 commits into from
Feb 7, 2016

Conversation

justin808
Copy link
Member

Review on Reviewable

@justin808
Copy link
Member Author

@samnang @robwise Thoughts?

@justin808 justin808 force-pushed the justin/improve-ensure-assets-compiled branch 2 times, most recently from e0276af to 689ed80 Compare February 7, 2016 04:39
@samnang
Copy link
Contributor

samnang commented Feb 7, 2016

lib/react_on_rails.rb, line 16 [r2] (raw file):
As I remember, invoke is most common method for callable objects in Java or C#, but it's not so common in Ruby community.

Comments from the review on Reviewable.io

@justin808
Copy link
Member Author

Review status: 0 of 5 files reviewed at latest revision, 1 unresolved discussion.

lib/react_on_rails.rb, line 16 [r2] (raw file):
ReactOnRails.ensure_assets_compiled looks better.

Agree?

Comments from the review on Reviewable.io

@justin808
Copy link
Member Author

This ONE is READY....

@robwise
Copy link
Contributor

robwise commented Feb 7, 2016

Reviewed 20 of 21 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

node_package/src/StoreRegistry.js, line 76 [r3] (raw file):
What's this file?

Comments from the review on Reviewable.io

@justin808
Rework files for asset checking
3f73cc6 
* Moved files under ReactOnRails::TestHelper
* Improved printouts
* Fixed issue if file touched outside of what webpack detects
* Refactored to reduce complexity lint error
* Moved logic in TestHelper to method object
@justin808 justin808 force-pushed the justin/improve-ensure-assets-compiled branch from 881461e to 3f73cc6 Compare February 7, 2016 22:59
justin808 added a commit that referenced this pull request Feb 7, 2016
@justin808
Merge pull request #258 from shakacode/justin/improve-ensure-assets-c…
6b5ae7d 
…ompiled

Rework files for asset checking
@justin808 justin808 merged commit 6b5ae7d into rob/improve-ensure-assets-compiled Feb 7, 2016
@justin808 justin808 deleted the justin/improve-ensure-assets-compiled branch February 7, 2016 23:07
@justin808
Copy link
Member Author

See #253

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@justin808 @samnang @robwise