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

Only run Coveralls when token exists #402

Merged
merged 3 commits into from Dec 7, 2018
Merged

Conversation

whymarrh
Copy link
Contributor

@whymarrh whymarrh commented Dec 7, 2018

Fixes #359

This PR updates the coveralls npm script to only run Coveralls when the auth token exists in the environment .

I've also removed the unused Travis CI configuration since that's confusing.

@holgerd77
Copy link
Member

Can you do work directly on the main repo? Then it gets possible that a reviewer can eventually directly rebase.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Looks good.

@holgerd77 holgerd77 merged commit e223587 into ethereumjs:master Dec 7, 2018
@whymarrh whymarrh deleted the fix-359 branch December 7, 2018 22:39
@whymarrh
Copy link
Contributor Author

whymarrh commented Dec 7, 2018

Can you do work directly on the main repo? Then it gets possible that a reviewer can eventually directly rebase.

Sure thing, I'll use the main repo for the smaller PRs. That said, I'm more than happy to rebase before merging.

@holgerd77
Copy link
Member

I think this also makes sense for the larger PRs, we often had the case that someone starts on a feature and then someone else is picking up on the work (due to time constraints of the other person e.g.). Or do you have got some more points on doing this from an own fork?

@whymarrh
Copy link
Contributor Author

whymarrh commented Dec 8, 2018

I'm not picky. I can work out of the main repo for all PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Coveralls CI run fails for PRs from forks
3 participants