Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

chore: Clean up CI scripts #230

Closed
wants to merge 5 commits into from
Closed

chore: Clean up CI scripts #230

wants to merge 5 commits into from

Conversation

jawnsy
Copy link
Contributor

@jawnsy jawnsy commented Jan 25, 2021

  • Build with Go provided by GitHub instead of building inside a Dockerfile, which should speed up the build
  • Condense build steps to avoid repeatedly downloading cached Go packages, which should speed up the build
  • Run goimports using golangci-lint, rather than running gofmt and goimports manually in make fmt
  • Skip signing/notarizing Darwin binary in build, since this takes a long time and is done in release
  • Only build/test for pull requests against master
  • Remove unnecessary steps in ci/image/Dockerfile
    • goimports is managed by golangci-lint
    • grep is already installed in the base image

@jawnsy jawnsy requested review from kylecarbs and cmoog January 25, 2021 01:43
@jawnsy jawnsy self-assigned this Jan 25, 2021
@jawnsy jawnsy requested a review from tychoish January 25, 2021 01:43
@jawnsy jawnsy force-pushed the update-build branch 3 times, most recently from 4f289d9 to 17cd0ba Compare January 25, 2021 02:09
@jawnsy
Copy link
Contributor Author

jawnsy commented Jan 25, 2021

@cmoog Some small cleanups to the build here, but I'm not really sure if it's an improvement. What do you think?

@cmoog
Copy link
Contributor

cmoog commented Jan 25, 2021

I could go either way with these changes. I would still like to run the notarized build on master to ensure we don't get surprised with errors when releases are tagged.

@jawnsy jawnsy force-pushed the update-build branch 2 times, most recently from 335d5d9 to 97bb75d Compare February 12, 2021 05:00
* Skip signing and notarizing Darwin binary in pull requests,
  since this takes a long time and is done in the master build
* Avoid uploading built artifacts for pull request builds
* Only build/test for pull requests against master
@jawnsy
Copy link
Contributor Author

jawnsy commented Feb 15, 2021

The useful parts of this change were merged in #243, obsoleting this PR

@jawnsy jawnsy closed this Feb 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants