search

blablacar / dgr

Container build and runtime tool
Apache License 2.0
248 stars 21 forks source link

make early quality checks part of the CI pipeline #223

Closed mark-kubacki closed 7 years ago

mark-kubacki commented 7 years ago

This commit brings dgr to current industry best-practices.

There is no point in building a commit that will be discarded later based on these easy to catch formalities.

Output of golint is not used to determine if the build succeeded.

mark-kubacki commented 7 years ago

The PR itself is fine, and uncovered a minor style flaw and something that seems to be a bug in waiting: https://travis-ci.org/blablacar/dgr/builds/179754485#L398

n0rad commented 7 years ago

./gomake quality is doing it

mark-kubacki commented 7 years ago

n0rad, yes I did read gomake. The point is to catch the before any build. Usually a CI pipeline stops at the first error, and catching those early allows for a rapid turnaround.

By the way, usually those are run parallel to any building, or at least before.

I hope you see its merits.

n0rad commented 7 years ago

but gomake's fmt seems to miss some files compare to yours

mark-kubacki commented 7 years ago

Here is an example: :-)

https://hub.blitznote.com/mark/caddy/pipelines/101

n0rad commented 7 years ago

true, fmt should follow the same lifecyle as goimports, and should be run during build.

I don't want to run it separatly, go project build is fast enough to integrate it direclty in the build process.

n0rad commented 7 years ago

all those tools are included in gomake. With gomake all code modification tools are run during build phase and all quality tools are run during quality phase.

All tools included, it takes ~2sec. So I don't want to seperate it from the build. As a contrary I think that at some point quality run should break directly the build to force dev to improve quality. this will require gomake improvement.

mark-kubacki commented 7 years ago

Then we have an agreement indeed.

It's just you use all the tools and processes in an uncommon, individualistic way. For example, all other projects which I have encountered run formal checks as separate step in their build pipeline. The order usually is ⊆: • formal checks (lint, vet…) || unit tests → integration tests → business tests → load test

On the other hand, you seem to be closing PRs right away, but keep issues open not till they are fixed, but until the release that carries the fix is out.

It's just a different perspective. No complaint, just a culture shock. So we better understand each other.

pipeline-example