microsoft / AL-Go

The plug-and-play DevOps solution for Business Central app development on GitHub
MIT License
253 stars 115 forks source link

Use PR if direct commit is not allowed #1085

Closed freddydk closed 1 month ago

freddydk commented 1 month ago

If Update AL-Go System Files is running on a schedule, use a PR if PRs are mandatory due to branch rules. Also name the PR with the templateSha, and check whether a PR with the same templateSha already exists.

If a PR exists, the workflow will issue a warning: image

The fix is done everywhere, where you can do a direct commit or a PR. If you have selected Direct Commit - it will first try Direct Commit and if that fails - it will issue a warning and try to create a PR.

All other workflows (than Update AL-Go System Files) will not check whether an existing PR already exists.

Fixes #1084

freddydk commented 1 month ago

Not a big fan of the separate tasks as there will be a lot of overhead if all steps run (e.g. checking out AL-Go repo, checking for updates, etc.). I think CheckForUpdates action should handle it in one go.

Not sure I understand what you mean by overhead - there are three steps for running upgrade:

  1. runs if the workflow is invoked manually
  2. runs if the workflow is invoked by a schedule (same behavior as before this PR)
  3. runs if the workflow is invoked by a schedule and 2. fails (instead of just failing really)
mazhelez commented 1 month ago

Not a big fan of the separate tasks as there will be a lot of overhead if all steps run (e.g. checking out AL-Go repo, checking for updates, etc.). I think CheckForUpdates action should handle it in one go.

Not sure I understand what you mean by overhead - there are three steps for running upgrade:

  1. runs if the workflow is invoked manually
  2. runs if the workflow is invoked by a schedule (same behavior as before this PR)
  3. runs if the workflow is invoked by a schedule and 2. fails (instead of just failing really)

If step 2. fails then step 3. will have to determine again whether there are updates (download the template, read the settings, etc.), when everything is already there.

I understand Alexander also meant something like the following:

try {
    push
}
catch {
    # pushing failed, trying PR
    create branch
    create PR
}
freddydk commented 1 month ago

I understand Alexander also meant something like the following:

try {
    push
}
catch {
    # pushing failed, trying PR
    create branch
    create PR
}

I will see if this can be done without any large refactoring