-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Build: simplify make targets to just run every time #5411
Conversation
…run them every time. Signed-off-by: Edward Welch <edward.welch@grafana.com>
Signed-off-by: Edward Welch <edward.welch@grafana.com>
…o versions 13/14/15 Signed-off-by: Edward Welch <edward.welch@grafana.com>
…I guess this all happens in the same place at the same time in drone. Signed-off-by: Edward Welch <edward.welch@grafana.com>
Signed-off-by: Edward Welch <edward.welch@grafana.com>
c91f888
to
98cea8c
Compare
@@ -353,7 +353,7 @@ local manifest(apps) = pipeline('manifest') { | |||
image: 'koalaman/shellcheck-alpine:stable', | |||
commands: ['apk add make bash && make lint-scripts'], | |||
}, | |||
make('loki', container=false) { depends_on: ['clone'] }, | |||
make('loki', container=false) { depends_on: ['clone', 'check-generated-files'] }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is necessary because previously we ran check-generated-files
at the same time as make loki
, the former runs a clean operation which removes the protos and then the make loki
would fail because the protos were missing. This worked previously because the make loki
step had a dependency to generate the protos and this PR removes that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Thanks @slim-bean ❤️
What this PR does / why we need it:
If I had a dollar for every time we had build failures related to file modified times and
make
.....This PR removes any dependant targets from our binary build commands such that they run every time, I'm tired of chasing down weird modified time bugs related to git checkouts and file changes because git doesn't store timestamps and thus this functionality in make is extremely brittle and wastes more time than it ever saved.
We have separate checks in our CI to make sure generated assests are regenerated if they are changed, we don't need to do that as part of something like
make loki
NOTE: While troubleshooting what turned out to be a drone issue, I also removed all the
mod=vendor
stuff we had in place to work through the migration of go from versions 1.13-1.16 which I'm about 99% sure isn't necessary anymore.Special notes for your reviewer:
Checklist
CHANGELOG.md
about the changes.