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

Added Travis CI #17

Merged
merged 7 commits into from Nov 10, 2018

Conversation

Projects
None yet
3 participants
@Mera-Gangapersaud
Copy link
Owner

commented Nov 10, 2018

I added the travis.yml file and the build passes! #5
image

currently it just tests npm test and npm run build and does not include a deploy section since this repo hasn't yet been deployed.

@SeanPrashad SeanPrashad self-requested a review Nov 10, 2018

@Mera-Gangapersaud Mera-Gangapersaud requested a review from klymenkoo Nov 10, 2018

@klymenkoo
Copy link
Collaborator

left a comment

Looks good to me!
The only suggestion from me is removing package-lock.json from the PR.

@Mera-Gangapersaud Mera-Gangapersaud force-pushed the TravisCI-intergration branch from b426b19 to 5491631 Nov 10, 2018

@Mera-Gangapersaud

This comment has been minimized.

Copy link
Owner Author

commented Nov 10, 2018

@klymenkoo removing the package-lock.json makes the build fail in Travis:

image

I believe we should have included the package-lock from this originally PR but for now we can still include it in this PR.

Mera-Gangapersaud added some commits Nov 10, 2018

@SeanPrashad
Copy link
Contributor

left a comment

This is a superb start to automate our build process - we should look into writing some tests for the UI once this lands. 🏝

I've requested a few tiny updates that will help us be more consistent with the prerequisites that we outline in our README. Let me know if you have any questions!

@@ -0,0 +1,9 @@
language: node_js
node_js:
- 'stable'

This comment has been minimized.

Copy link
@SeanPrashad

SeanPrashad Nov 10, 2018

Contributor

Let's also include the lts/* and 10.10 version of Node. The former represents the latest long-term support of Node (think of it like Microsoft supporting Windows 7 until 20XX).

README.md Outdated
@@ -1,6 +1,6 @@
# Creative-Collab

[![code style: prettier](https://img.shields.io/badge/code_style-prettier-ff69b4.svg?style=flat-square)](https://github.com/prettier/prettier)
[![Build Status](https://travis-ci.com/Mera-Gangapersaud/Creative-Collab.svg?branch=master)](https://travis-ci.com/Mera-Gangapersaud/Creative-Collab) [![code style: prettier](https://img.shields.io/badge/code_style-prettier-ff69b4.svg?style=flat-square)](https://github.com/prettier/prettier)

This comment has been minimized.

Copy link
@SeanPrashad

SeanPrashad Nov 10, 2018

Contributor

nit: one space between each badge please 🙏🏽🏵

- node_modules
script:
- npm test
- npm run build

This comment has been minimized.

Copy link
@SeanPrashad

SeanPrashad Nov 10, 2018

Contributor

It might also be useful to add the following section above the script section:

before_install:
  - npm i -g npm@6.4.1

as we specify the npm version to use in our README and CONTRIBUTING.md 📝

@SeanPrashad
Copy link
Contributor

left a comment

LGTM 🚢 it!

@SeanPrashad SeanPrashad merged commit 15e0c98 into master Nov 10, 2018

2 checks passed

Travis CI - Branch Build Passed
Details
Travis CI - Pull Request Build Passed
Details

@SeanPrashad SeanPrashad deleted the TravisCI-intergration branch Nov 10, 2018

@SeanPrashad SeanPrashad referenced this pull request Nov 10, 2018

Closed

Continuous Integration #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.