search

canonical / grafana-agent-snap

Snap of Grafana Agent, a telemetry collector for sending metrics, logs and traces to the Grafana's LGTM stack.
https://snapcraft.io/grafana-agent
Apache License 2.0
1 stars 5 forks source link

feat: add test builds of the snap on pull_request events #64

Closed ca-scribner closed 2 months ago

ca-scribner commented 3 months ago

Adds test builds to assert the snap is buildable before changes are merged into main. This will help identify future build failures like when this change was merged before merging to main, and will give automated builds some CI to run against

OPENG-2388

ca-scribner commented 2 months ago

@sed-i I agree with you that it would be nice to deduplicate, but I'm not sure if the available options are better.

If we merge this into the "release" CI by adding if statements, would it feel strange that a pull_request event has a release CI run that passes but does not actually release anything?

If we put the common parts in a reusable _build_snap workflow, _build_snap would run as a separate job (eg: separate runners). The publish CI needs the .snap artifact produced by _build_snap, so we'd need to include upload-artifact/download-artifact steps to pass the artifact. Since _build_snap only reuses ~15 lines of code, the new upload/download code will be about as much as we save. So I'm not sure its worth it

tl;dr: I'm happy to do anything (all in release.yaml, make a _build_snap.yaml, or leave as is), but I don't have a strong preference on which :) wdyt?

ca-scribner commented 2 months ago

I poked @lucabello about this offline and I think he's roughly in the same "we could do whatever" space. He raised how snap building will sometime be replaced by a central build CI too, so whatever we do here will likely get overwritten soonish

ca-scribner commented 2 months ago

@sed-i and I discussed this offline. We decided on leaving the duplication as is, but to add a comment at the top of the workflows so people know if they're editing one they should edit both.