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

[#9970] Fix failing Travis build #9971

Merged
merged 6 commits into from
Feb 28, 2020
Merged

Conversation

jtankw3
Copy link
Contributor

@jtankw3 jtankw3 commented Feb 27, 2020

Fixes #9970

Outline of Solution

  • Remove npm update and change npm install to npm ci in .travis.yml
  • commit package-lock.json to the repo by editing .gitignore
  • Edit setting up guide to change npm install to npm ci

Copy link
Contributor

@madanalogy madanalogy left a comment

Choose a reason for hiding this comment

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

Hello, thanks for contributing to Teammates!

If you would like to take the approach of converting npm install to npm ci, please also update the setup guide. Otherwise an alternative approach would simply be to remove npm update as the line has been made redundant since #9783.

Copy link
Contributor

@madanalogy madanalogy left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@wkurniawan07 wkurniawan07 left a comment

Choose a reason for hiding this comment

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

There should be a line in .travis.yml that specifies that node_modules is cached. npm ci will make that operation redundant, so can you also remove that line?

.travis.yml Outdated

jobs:
include:
- stage: "Prepare Dependencies"
name: "Prepare Dependencies"
script:
- ./gradlew downloadDependencies downloadTestDependencies downloadLinters
- npm install
- npm update
- npm ci
Copy link
Member

Choose a reason for hiding this comment

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

You can remove this line entirely, because this stage is meant to be caching dependencies.

Copy link
Member

@wkurniawan07 wkurniawan07 left a comment

Choose a reason for hiding this comment

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

LGTM

@wkurniawan07 wkurniawan07 merged commit 7147f22 into TEAMMATES:master Feb 28, 2020
tiuweehan added a commit to tiuweehan/teammates that referenced this pull request Feb 28, 2020
* master:
  [TEAMMATES#9382] Add MasqueradeModeService Tests (TEAMMATES#9955)
  [TEAMMATES#9302] Support rich text for MCQ option text (TEAMMATES#9944)
  [TEAMMATES#9642] Feedback sessions with # or ? in their name can't be edited. (TEAMMATES#9925)
  [TEAMMATES#9970] Fix failing Travis build (TEAMMATES#9971)
@wkurniawan07 wkurniawan07 added this to the V7.0.0-beta.0 milestone Mar 15, 2020
@jtankw3 jtankw3 deleted the fix-travis branch August 14, 2020 03:11
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.

Fix failing Travis build
3 participants