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

Enable continuous integration #94

Merged
merged 1 commit into from Mar 21, 2017
Merged

Enable continuous integration #94

merged 1 commit into from Mar 21, 2017

Conversation

mathiasbynens
Copy link
Contributor

@mathiasbynens mathiasbynens commented Apr 9, 2015

With this change, every commit pushed to master and every pull request received triggers a build. If anything breaks, e.g. if the MD5 hashes for generated ROMs are incorrect, it automatically reports an error straight in the GitHub pull request UI.

Example build log: https://travis-ci.org/PoCs/pokered/builds/151586998

For this to work, you’d have to enable Travis for this repository: https://docs.travis-ci.com/user/getting-started/#To-get-started-with-Travis-CI%3A

@kanzure
Copy link
Member

kanzure commented Apr 9, 2015

What's check_status for, here?

@mathiasbynens
Copy link
Contributor Author

It would detect edge cases where running make alters a Git-tracked file. That could happen if someone tries to sneak in a commit that, when running make afterwards, changes the generated 2bpp or pic files, for example.

Consider my patch in #92: at the moment, the only way to verify that it doesn’t mess anything up is by merging it locally, running make, and confirming there are no uncommitted changes. But with this change, that check would happen automatically as soon as anyone submits a PR.

@mathiasbynens
Copy link
Contributor Author

Can someone review this please?

@dannye
Copy link
Member

dannye commented Mar 2, 2016

I'm not speaking for everybody, but I don't think this is in our interest. Our makefile already verifies the output rom.
The other features of Travis - automatically posting to the Github PR, detect when make alters tracked files - just don't seem very necessary.

@bgourlie
Copy link

Considering how low-lift adding CI to this project would be, I don't understand how anyone could argue against it.

@kanzure
Copy link
Member

kanzure commented Aug 11, 2016

ACK, this looks reasonable to me. I like the idea of introducing continuous integration to the project and its users, who may be unfamiliar with the benefits of regular testing and builds. Let's do this.

@Sanqui
Copy link
Member

Sanqui commented Mar 20, 2017

Is anybody still opposed to this? I've updated (Sanqui@d7bd7f4) the Travis script and I'm ready to set it up.

@Sanqui Sanqui merged commit defd8ad into pret:master Mar 21, 2017
@Sanqui
Copy link
Member

Sanqui commented Mar 21, 2017

Improved and accepted in #148, thank you!

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

5 participants