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

ci.sh can run cargo-audit #6549

Merged
merged 3 commits into from Nov 30, 2018

Conversation

Projects
None yet
3 participants
@illicitonion
Copy link
Contributor

illicitonion commented Sep 24, 2018

See https://github.com/rustsec/cargo-audit for details

Nothing actually runs this yet, I'm going to experiment with travis cron jobs

@illicitonion illicitonion requested a review from stuhood Sep 24, 2018

@stuhood

This comment has been minimized.

Copy link
Member

stuhood commented Sep 24, 2018

Seems like actually adding it with the travis config that will run it would be worthwhile?

@stuhood
Copy link
Member

stuhood left a comment

Thanks.

.travis.yml Outdated
@@ -84,11 +88,19 @@ default_test_config: &default_test_config
- sudo rm -rf /usr/lib/jvm/java-6-openjdk-amd64
- jdk_switcher use oraclejdk8

with_fuse: &with_fuse
before_install:

This comment has been minimized.

@stuhood

stuhood Sep 25, 2018

Member

I've never seen this before! I think yaml doesn't like tabs.

.travis.yml Outdated
before_script:
- ulimit -c unlimited
- ulimit -n 8192
script:
- ./build-support/bin/travis-ci.sh -bs

#- if: type == "cron"

This comment has been minimized.

@stuhood

stuhood Sep 25, 2018

Member

Whoa... is this syntax? A comment above this explaining that the next line is a magic comment would be good.

This comment has been minimized.

@baroquebobcat

baroquebobcat Sep 29, 2018

Contributor

I think this is just a comment. If so, maybe xx it?

@baroquebobcat
Copy link
Contributor

baroquebobcat left a comment

Nice. Looks good to me apart from two minor inline comments and the below.

I think it'd be nice to have a short comment on the new mixins that says what they're for. runs_on_ci is pretty self explanatory, but it might make sense to have a little more description for with_fuse.

.travis.yml Outdated
before_script:
- ulimit -c unlimited
- ulimit -n 8192
script:
- ./build-support/bin/travis-ci.sh -bs

#- if: type == "cron"

This comment has been minimized.

@baroquebobcat

baroquebobcat Sep 29, 2018

Contributor

I think this is just a comment. If so, maybe xx it?

(
"${REPO_ROOT}/build-support/bin/native/cargo" ensure-installed --package=cargo-audit --version=0.5.2
"${REPO_ROOT}/build-support/bin/native/cargo" audit -f "${REPO_ROOT}/src/rust/engine/Cargo.lock"
) || die "Pants clippy failure"

This comment has been minimized.

@baroquebobcat

baroquebobcat Sep 29, 2018

Contributor

nit: s/Pants clippy failure/Pants cargo-audit failure/

This comment has been minimized.

@stuhood

stuhood Nov 28, 2018

Member

^ This is still a thing.

@illicitonion illicitonion force-pushed the twitter:dwagnerhall/cargo-audit branch 2 times, most recently from 9396479 to fca4647 Nov 28, 2018

@illicitonion

This comment has been minimized.

Copy link
Contributor

illicitonion commented Nov 28, 2018

Ok, I think this should actually work now. Any last looks? :)

(
"${REPO_ROOT}/build-support/bin/native/cargo" ensure-installed --package=cargo-audit --version=0.5.2
"${REPO_ROOT}/build-support/bin/native/cargo" audit -f "${REPO_ROOT}/src/rust/engine/Cargo.lock"
) || die "Pants clippy failure"

This comment has been minimized.

@stuhood

stuhood Nov 28, 2018

Member

^ This is still a thing.

@stuhood

This comment has been minimized.

Copy link
Member

stuhood commented Nov 28, 2018

A semi-orthogonal comment: it would be nice to run both clippy and cargo audit as part of the commit hook... a fair amount of stuff is caught there, but not seeing clippy until a CI run is mostly finished has been painful.

illicitonion added some commits Sep 24, 2018

ci.sh can run cargo-audit
See https://github.com/rustsec/cargo-audit for details

Nothing actually runs this yet, I'm going to experiment with travis cron jobs

@illicitonion illicitonion force-pushed the twitter:dwagnerhall/cargo-audit branch from fca4647 to f4cde76 Nov 28, 2018

@illicitonion illicitonion merged commit 5d76d0b into pantsbuild:master Nov 30, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@illicitonion illicitonion deleted the twitter:dwagnerhall/cargo-audit branch Nov 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment