-
Notifications
You must be signed in to change notification settings - Fork 2
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
Continuous Integration #2
base: master
Are you sure you want to change the base?
Conversation
First, the RFC (Request for Comment) - very cool. Seems pretty straight forward to read - I hope to create one soon. The length was great and your description was very helpful. In terms of the CI (Continuous Integration) the first pass looks amazing!! How does it know what to do to test things? Do you (we) have to set that up or does Travis just know how to do it? |
@danzen Travis provides a build environment that is mostly managed from the .travis.yml file: There are plenty of instructions on building javascript/node projects here: https://docs.travis-ci.com/user/languages/javascript-with-nodejs/ Other than adding the travis file to the repo, each project needs to enabled within the travis website. To do this, GitHub permissions are needed to access the project. If this RFC is accepted by @gskinner, then someone would need to enable Travis for us, or provide permission to do so ourselves. I was able to enable this on my fork of EaselJS, as I am an owner of that fork. |
Does anyone have experience with GitHub actions? Would that somehow be more suitable over TravisCI? |
|
||
1. Cross-browser Javascript test suite | ||
2. Test coverage checking | ||
3. Static code linting tooling with ESLint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't state which ESLint rules to apply, or how they would be applied.
To make this work easier to digest+decide, I'll this out to it's own RFC....and probably the documentation coverage checking it also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lint rules might be tricky so I think it's a good idea to make a separate RFC. I ran the combined build against the rules I use and a lot of things came up, such as extensive use of '==' instead of '==='.
I think the temptation would be to just adjust the rules so that the current build passes.
Here's my request for comments on setting up continuous integration with CreateJS.
Note this is the first RFC of it's kind. Happy to hear thoughts on this process also. Refer to the main README for more info on this RFC process: https://github.com/ReCreateJS/rfcs#createjs-rfcs
Accordingly , this RFC is now at the 'Build consensus and integrate feedback' stage:
The idea is to get feedback on what's written, and once consensus is reached, then we hit the merge button, and this becomes and offical piece of work. Say whatever you want, and I'd even say include any thoughts on whether the RFC includes too much or too little.
I don't think a RFC implies that all the work needs to be done at once. It just gets approved, and then the work could perhaps be broken down into various tasks that can get done and merged independently, but can all link back to this RFC.
Of course we'd need @gskinner to be involved to see the work merged into the actual repo, but this should also demonstrate how RFCs work and can be implemented.
I've started an initial PR for running tests on Travis: CreateJS/EaselJS#1043
How to review? Click the 'Files changed' tab, and you can start a review, adding comments in the changed files wherever you want. OR in the same area you can just click a line and make a comment.