chore: improve blockifier CI trigger and scope

[dorimedini-starkware]

Pull Request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Build-related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this introduce a breaking change?

• Yes
• No

Other information

---

This change is 

[dorimedini-starkware]

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @noaov1)

---

.github/workflows/blockifier_coverage.yml line 0 at r2 (raw file):
note to self:
is this CI phase even needed?
don't we have a codecov phase in the main CI?

[noaov1]

Reviewed 1 of 2 files at r2, all commit messages.
Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware)

---

.github/workflows/blockifier_ci.yml line 19 at r2 (raw file):

      - 'crates/native_blockifier/**'
      - 'scripts/install_build_tools.sh'
      - 'scripts/requirements.txt'

Do we want this requirement file or the one under crates/blockifier/tests?

Code quote:

      - 'scripts/requirements.txt'

---

.github/workflows/blockifier_compiled_cairo.yml line 10 at r2 (raw file):

    - v[0-9].**
    paths:
    - '.github/workflows/blockifier_compiled_cairo.yml'

What does it mean to have the file name in the file itself?

Code quote:

    - '.github/workflows/blockifier_compiled_cairo.yml'

[dorimedini-starkware]

Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware and @noaov1)

---

.github/workflows/blockifier_ci.yml line 19 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Do we want this requirement file or the one under crates/blockifier/tests?

good catch.. this is actually only needed in the blockifier_compiled_cairo job below

---

.github/workflows/blockifier_compiled_cairo.yml line 10 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

What does it mean to have the file name in the file itself?

so this job triggers when it changes (I think this is what it does, at least)

[dorimedini-starkware]

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @noaov1)

---

.github/workflows/blockifier_coverage.yml line at r2 (raw file):

Previously, dorimedini-starkware wrote…

note to self:
is this CI phase even needed?
don't we have a codecov phase in the main CI?

related PR

[noaov1]

Reviewed 2 of 4 files at r1, 1 of 2 files at r2, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @dorimedini-starkware)

---

.github/workflows/blockifier_ci.yml line 11 at r4 (raw file):

      - main-v[0-9].**
    tags:
      - v[0-9].**

Why is there no paths here?

Code quote:

  push:
    branches:
      - main
      - main-v[0-9].**
    tags:
      - v[0-9].**

---

.github/workflows/blockifier_ci.yml line 27 at r4 (raw file):

      - 'build_native_blockifier_in_docker.sh'
      - 'crates/blockifier/**'
      - 'crates/native_blockifier/**'

Why is the native blockifier trigger this ci job?

Code quote:

      - 'crates/native_blockifier/**'

---

.github/workflows/blockifier_ci.yml line 29 at r4 (raw file):

      - 'crates/native_blockifier/**'
      - 'scripts/build_native_blockifier.sh'
      - 'scripts/install_build_tools.sh'

What is this file?

Code quote:

      - 'scripts/install_build_tools.sh'

---

.github/workflows/blockifier_compiled_cairo.yml line 6 at r4 (raw file):

  push:
    branches:
    - main

Why here we don't also have: main-v[0-9].**?
Do we want to add this branch to the papyrus ci?

Code quote:

    branches:
    - main

---

.github/workflows/blockifier_coverage.yml line 9 at r4 (raw file):

      - 'crates/blockifier/**'
      - 'crates/native_blockifier/**'
  push:

Why here we don't need to mention branches?

Code quote:

  push:

---

.github/workflows/blockifier_post-merge.yml line 11 at r4 (raw file):

      - 'crates/blockifier/**'
      - 'crates/native_blockifier/**'
      - 'scripts/requirements.txt'

Why?

Code quote:

      - 'scripts/requirements.txt'

[dorimedini-starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware and @noaov1)

---

.github/workflows/blockifier_ci.yml line 11 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why is there no paths here?

until we decide how we fix the brokefier and not create many redundant artifacts, we will temporarily build a native blockifier artifact on every merge to a release branch (on push to branches: main, main-v[0-9].**).
so, no paths: always trigger this job

---

.github/workflows/blockifier_ci.yml line 27 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why is the native blockifier trigger this ci job?

for the native-blockifier-artifacts-push job in this file

---

.github/workflows/blockifier_ci.yml line 29 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

What is this file?

part of the build-native-blockifier process.
script runs docker -> docker calls script to install deps -> script is install_build_tools

---

.github/workflows/blockifier_compiled_cairo.yml line 6 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why here we don't also have: main-v[0-9].**?
Do we want to add this branch to the papyrus ci?

1. good catch, thanks! added
2. yes, which papyrus CIs don't have it?

---

.github/workflows/blockifier_coverage.yml line 9 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why here we don't need to mention branches?

I think code coverage report is really only useful during review; "post-merge coverage report" sounds redundant, WDYT?

---

.github/workflows/blockifier_post-merge.yml line 11 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why?

my mistake
need the requirements.txt in the blockifier crate (blockifier/** covers it)

[noaov1]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dorimedini-starkware)

---

.github/workflows/blockifier_ci.yml line 11 at r4 (raw file):

Previously, dorimedini-starkware wrote…

until we decide how we fix the brokefier and not create many redundant artifacts, we will temporarily build a native blockifier artifact on every merge to a release branch (on push to branches: main, main-v[0-9].**).
so, no paths: always trigger this job

Got it. Document?
Why not have this job in a native_blockifier_ci.yml file?

---

.github/workflows/blockifier_ci.yml line 29 at r4 (raw file):

Previously, dorimedini-starkware wrote…

part of the build-native-blockifier process.
script runs docker -> docker calls script to install deps -> script is install_build_tools

Document?

---

.github/workflows/blockifier_compiled_cairo.yml line 6 at r4 (raw file):

Previously, dorimedini-starkware wrote…

1. good catch, thanks! added
2. yes, which papyrus CIs don't have it?

I see it's missing in:
papyrus_ci.yml, papyrus_docker-publish.yml and helm-install.yml

---

.github/workflows/blockifier_coverage.yml line 9 at r4 (raw file):

Previously, dorimedini-starkware wrote…

I think code coverage report is really only useful during review; "post-merge coverage report" sounds redundant, WDYT?

I agree. What will adding branches do? Trigger this job post merge to the mentioned branches?

[dorimedini-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @noaov1)

---

.github/workflows/blockifier_ci.yml line 11 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Got it. Document?
Why not have this job in a native_blockifier_ci.yml file?

1. done
2. already have a TODO for that at the top of the file :)

---

.github/workflows/blockifier_compiled_cairo.yml line 6 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I see it's missing in:
papyrus_ci.yml, papyrus_docker-publish.yml and helm-install.yml

out of scope of this PR, but we should fix this

[dorimedini-starkware]

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @noaov1)

---

.github/workflows/blockifier_coverage.yml line 9 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I agree. What will adding branches do? Trigger this job post merge to the mentioned branches?

oops... this was run on every push to any branch...
deleted the push event

[noaov1]

Reviewed 2 of 2 files at r6.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @dorimedini-starkware)

---

.github/workflows/blockifier_coverage.yml line 9 at r4 (raw file):

Previously, dorimedini-starkware wrote…

oops... this was run on every push to any branch...
deleted the push event

Why don't we want it to run on every push to the (native ) blockifier branch?

[noaov1]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

[dorimedini-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

---

.github/workflows/blockifier_coverage.yml line 9 at r4 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why don't we want it to run on every push to the (native ) blockifier branch?

what is the native blockifier branch?
if you are asking why not run on every push to main/main-v0.13.2 - why run on every push? all you get is a post-merge report in your inbox; IMO running jobs on push events is only useful (if even) when the failure indicates an error in the code, like test failure or artifact build failure. WDYT?

In any case I think this particular discussion is a bit redundant since codecov should be moving to the main CI anyway

[dorimedini-starkware]

Reviewed 4 of 4 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)