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 ci build failures #166

Merged
merged 10 commits into from
Dec 3, 2015
Merged

Fix ci build failures #166

merged 10 commits into from
Dec 3, 2015

Conversation

dylangrafmyre
Copy link
Contributor

@justin808, @robwise, @alexfedoseev
I was able to fix the failing Travis CI build with setting Travis to install package g++-4.9.

However, I fixed the failing Codeship build by setting react/prop-types: 0 within .eslintrc.

This was the build fail issue:

1__dylan_dylans-macbook-pro____react-webpack-rails-tutorial_client__zsh_

And referenced here in the issues for eslint-plugin-react:
jsx-eslint/eslint-plugin-react#9

I am sure you will not want to have this rule continually ignored; therefore, someone will need to address the issue in the code?

Thanks,
Dylan

@@ -40,7 +40,7 @@ rules:
react/no-did-update-set-state: 0
react/no-multi-comp: 2
react/no-unknown-property: 2
react/prop-types: 1
react/prop-types: 0
Copy link
Member

Choose a reason for hiding this comment

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

This is a really useful validation. There's some regression in eslint that's failing this.

At the min, we need to doc why we're doing this change temporarily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@justin808 did you read the link in my above comment about the github issue from the eslint-plugin-react. Someone familiar with the react code should look at this and see if anything new in the code has triggered this issue? Or is still a bug in the plugin. Maybe you folks can try another react-props eslint plugin?

@josiasds josiasds mentioned this pull request Dec 3, 2015
@dylangrafmyre
Copy link
Contributor Author

@justin808 Thanks to @josiasds for debugging the the react code that was causing the eslint-plugin-react to fail. This resolved the issue without having to adjust the lint rule. If no other objections, This is ready to merge.

@josiasds
Copy link
Member

josiasds commented Dec 3, 2015

Still don't know why it was failing the linter though 😐

@josiasds
Copy link
Member

josiasds commented Dec 3, 2015

I think now we're good to merge this @dylangrafmyre @justin808

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.

None yet

3 participants