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

Fix #427 #464

Merged
merged 6 commits into from
May 31, 2017
Merged

Fix #427 #464

merged 6 commits into from
May 31, 2017

Conversation

Rokt33r
Copy link
Contributor

@Rokt33r Rokt33r commented Jan 19, 2017

I fixed #427 .

This PR must be reviewed. I'm also going to test this fork by my new project.

@Rokt33r
Copy link
Contributor Author

Rokt33r commented Jan 19, 2017

I confirmed it works.

rhl-fix

@Rokt33r Rokt33r changed the title [WIP] Fix #427 Fix #427 Jan 19, 2017
@calesce
Copy link
Collaborator

calesce commented Jan 22, 2017

Thanks @Rokt33r, looks great! I'll review more closely tomorrow. Apologies for the delay.

@@ -0,0 +1,3 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you call this directory something like arrow-function-in-constructor rather than issue-427? Also, we could probably use a few more test fixtures, see what we have for class properties.

I'd also like at least one AppContainer test, since those are more integration-level tests that verify that RHL works, not just the Babel plugin. Here's a reference example

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do gladly. But, I think I can do after the 2nd week of the next month.(I have to finish my graduate thesis.) Could you wait about 3 weeks?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem! Take your time.

@Rokt33r
Copy link
Contributor Author

Rokt33r commented Feb 10, 2017

Done! Please review this again. @calesce

@ThisIsRuddy
Copy link

Please merge :(

@montogeek
Copy link
Collaborator

@calesce It is ok to merge it?

@calesce
Copy link
Collaborator

calesce commented May 31, 2017

Yep, sorry for the delay, I burned out hard maintaining this project 😄

I only had issues with the file structure but the rest looked fine 👍

@calesce calesce merged commit 7ab86ed into gaearon:master May 31, 2017
@ThisIsRuddy
Copy link

❤️

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.

Will this convention be supported?
4 participants