search

status-im / status-desktop

Status Desktop client made in Nim & QML
https://status.app
Mozilla Public License 2.0
288 stars 78 forks source link

ci: improve master builds #11901

Open yakimant opened 1 year ago

yakimant commented 1 year ago

Description

Upd 17/08/2023: Added option 4

Situation

We have a job in Jenkins running build & e2e tests on each push to master: https://ci.infra.status.im/job/status-desktop/job/master/

We also run build & e2e on each PR branch push. Build failure blocks PR from merging, but being up-to-date with master is not required.

We also run e2e nightly on master: https://ci.infra.status.im/job/status-desktop/job/nightly-e2e/

Usage

Developers are not requiring PR branch to be up-to-date withmaster, because it becomes difficult to merge. But still they want to know, that master is not broken. Previously they've got notifications on Discord, but it's not the case with switch to Status. Currently noone is watching the results manually, unless maybe when master got broken for bisect.

Diff with mobile flow

  1. Mobile don't run e2e on each push, only on demand
  2. Mobile require PR to be up-to-date with master
  3. Thus they know for sure, that code doesn't break master
  4. And don't need master builds

Options

  1. Leave master build, enable notifications to Status
  2. Switch flow to Mobile-like
  3. Switch off master build
  4. Leave as is, useful for failure bisecting
osmaczko commented 1 year ago

I'll opt for Option 1.

For Option 2, as you mentioned, it will make PRs hard to merge.

Regarding Option 3, I believe pushes to master are much less frequent than pushes to certain PRs. Contributors often modify the code to address review suggestions and fix bugs found by QAs. In other words, removing the master build will not significantly improve the load on Jenkins' nodes.

There is also Option 4 - do nothing. Pros: Enables easier bisecting when things go sideways.. Cons: It has some impact on the load on Jenkins' nodes.

jrainville commented 1 year ago

We could do option 1 but if the load is too big for master builds, we could do it as a nightly instead, so we get one report each day.

Another improvement we could do that we discussed in the e2e channel a while ago was to do the e2e build only on demand, but still require it to pass to merge, without the need to rebase on master. Sometimes, we push to a PR and we're not fully done so running the e2e is useless.

yakimant commented 1 year ago

@osmaczko, regarding option 2: Mobile doesn't require e2e for each push and builds take 15min max. It's actually a good question if Mobile experience issues with merging. It would be nice to have a survey.

@jrainville, Option 1 doesn't cause any build load issues afaik. I like the idea of not running e2e on each push.