Skip to content

Conversation

@CarlosMecha
Copy link
Contributor

@CarlosMecha CarlosMecha requested review from a team and f2prateek June 29, 2018 22:44
@CarlosMecha CarlosMecha merged commit c876d4e into master Jun 29, 2018
@CarlosMecha CarlosMecha deleted the platform-2861 branch June 29, 2018 22:46
- run:
name: Run tests
command: make test
name: Authenticate npm

Choose a reason for hiding this comment

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

is this required if it is already done in the setup phase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we don't cache the npm config file (the one that gets updated with npm login). I'll ask security if we could do it.

name: Run coverage
command: make coverage
name: Coverage
command: yarn coverage

Choose a reason for hiding this comment

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

I think coverage should run after test?

jobs:
- setup
- test:
- conventions:

Choose a reason for hiding this comment

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

conventions sounds like a weird name for this. I think the jobs should be named for what they're doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's true, I'll open a new PR to address that. I'm going to add coverage and lint to test_phantomjs

nettofarah pushed a commit that referenced this pull request Feb 1, 2021
* elevio

* kenshoo-infinity

* optimizely

* segmentio

* taplytics
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.

3 participants