-
Notifications
You must be signed in to change notification settings - Fork 3.6k
TravisCI should help to discover eslint warnings. #800
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
Conversation
This would not only impact CI though. I don't have a strong opinion on whether they should all be warnings, all be errors or be a combination; I just wanted to point that out. |
This will be good in the long run for a cleaner code base. LGTM |
Weird travis passed but not appveyor! But LGTM. It's important we keep the code clean |
@mathieumg We anyway ask authors to address This will save their and our time in the long run 🍒 |
An interesting note about AppVeyor is that it is merging in master and testing that. Whereas travis does not. |
appveyor fails with this (already addressed in another commit) warning:
I merge. |
TravisCI should help to discover eslint warnings.
I was wondering too, I'm no CI expert. How does Travis proceed? A PR is only a patch in the end, it has to be applied on something to be whole and testable. I imagine this was an exceptional situation anyway, a PR which changes the conditions for a build to pass/fail submitted in-between two CI runs for another PR, which happens to break the build by the new conditions but not the old ones. |
😨 Bahh too much complexity.... I don't know the specifics, but I seem to recall that for every pull request github attempts to maintain two references to it. One which is the last commit of the PR and the second is the sha for a commit that would merge the two together. I believe you can fetch either of those commits to run tests against. This is a foggy recollection from something I read a long time ago. |
Oh, yeah, that's right. I checked out a PR once that way, had totally forgotten about it. It went something like |
Guess now is a good time for a quick plug to my blog. This is what I use to test PRs out locally: http://softwarebymatt.com/blog/git-pull-request-fetch/ |
Awesome, added to my aliases! 🍭 |
If you are not the owner of the project your I'm using There are |
Addresses such kind of things in review process:
#786 (comment)
#786 (comment)