Skip to content

Move lint jobs to CircleCI#2004

Merged
Simperfit merged 2 commits into
api-platform:2.2from
teohhanhui:move-lint-jobs-to-circleci
Jun 6, 2018
Merged

Move lint jobs to CircleCI#2004
Simperfit merged 2 commits into
api-platform:2.2from
teohhanhui:move-lint-jobs-to-circleci

Conversation

@teohhanhui

Copy link
Copy Markdown
Contributor
Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

@bendavies

bendavies commented Jun 5, 2018

Copy link
Copy Markdown
Contributor

you can add a php-cs-fixer fixer cache step

@soyuka soyuka mentioned this pull request Jun 5, 2018
@teohhanhui teohhanhui force-pushed the move-lint-jobs-to-circleci branch from 3b30a02 to 3d91257 Compare June 5, 2018 15:06
@teohhanhui

Copy link
Copy Markdown
Contributor Author

@bendavies Good catch! :D

@teohhanhui teohhanhui force-pushed the move-lint-jobs-to-circleci branch 2 times, most recently from 0e0ee87 to f388581 Compare June 5, 2018 15:39
Comment thread .circleci/config.yml
save_cache:
paths:
- .php_cs.cache
key: php-cs-fixer-cache-{{ .Branch }}-{{ .BuildNum }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

are you sure this is correct?
you never restore using .BuildNum so it seems pointless.
(i'm aware of how circle ci restore_cache works with substrings)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i thought you might
save key:

php-cs-fixer-cache-{{ .Branch }}-{{ .Revision }}

restore keys:

keys:
  - php-cs-fixer-cache-{{ .Branch }}-{{ .Revision }}
  - php-cs-fixer-cache-{{ .Branch }}
  - php-cs-fixer-cache

@teohhanhui teohhanhui Jun 5, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's intentional. The .BuildNum is for cache busting.

@bendavies bendavies Jun 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah, in my project i actually generate a hash of all php files they php-cs-fixer runs over, then cache on that.

something like:
find src tests -type f -name "*.php" -exec md5sum {} + > php-cs-fixer.checksum

- restore_cache:
  keys:
    - php-cs-fixer-{{ checksum "php-cs-fixer.checksum" }}
    - php-cs-fixer-
...
- save_cache:
  key: php-cs-fixer-{{ checksum "php-cs-fixer.checksum" }}
  paths:
    - .php_cs.cache

then there is no need for cache busting.

@bendavies

Copy link
Copy Markdown
Contributor

there is npm caching in here, but i can't see that npm is ever ran. am i missing it?

@teohhanhui

Copy link
Copy Markdown
Contributor Author

@bendavies npx ;)

@bendavies

Copy link
Copy Markdown
Contributor

ah! thanks

@teohhanhui teohhanhui force-pushed the move-lint-jobs-to-circleci branch from f388581 to 71724b4 Compare June 5, 2018 16:13
@teohhanhui

Copy link
Copy Markdown
Contributor Author

Overall build time should be back down to under 10 minutes (roughly). 🎉

@teohhanhui teohhanhui force-pushed the move-lint-jobs-to-circleci branch from 8e3b02e to b2a83fb Compare June 5, 2018 17:11
@Simperfit Simperfit merged commit 435d445 into api-platform:2.2 Jun 6, 2018
@Simperfit

Copy link
Copy Markdown
Contributor

Thanks @teohhanhui

@teohhanhui teohhanhui deleted the move-lint-jobs-to-circleci branch June 6, 2018 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants