Andrushika commented on code in PR #76839:
URL: https://github.com/apache/airflow/pull/76839#discussion_r4546912166
##########
dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md:
##########
@@ -109,6 +109,58 @@ Four pieces, in dependency order:
Plus `breeze ci audit` for the contributor's local setup, and documentation.
+### Pull request lifecycle
+
+What a gated pull request shows, and who acts next, from the moment it is
opened until it is merged
+or closed. The [state machine](#state-machine) below is the reconciler's view
of the same flow; this
+diagram is the contributor's and the maintainer's view. Pull requests of
committers, collaborators
+and exempted organisations are not gated and follow the usual project CI flow.
+
+```mermaid
+%%{init: {'theme': 'neutral'}}%%
+stateDiagram-v2
+ state "Waiting for the author: draft, fork CI not set up or no run for the
head commit" as waiting
+ state "Fork CI running" as running
+ state "Fork CI failed: out of the review queue" as failed
+ state "Green: ready for maintainer review, CI passed" as ready
+ state "Green, base too old: ready for maintainer review, CI passed pending
until a rebase" as rebase
+ state "Green, CI files changed: needs project ci" as needs_project_ci
+ state "Project CI: use project ci" as project_ci
+ state "Approved" as approved
+ state "Merged" as merged
+ state "Closed after 30 days without fork CI activity" as closed
+
+ [*] --> waiting: an external contributor opens a pull request
+ waiting --> running: the author enables the fork workflow and pushes
+ running --> failed: a job fails, times out or is cancelled, or the run
cannot start
+ running --> ready: the run passes on a current base
+ running --> rebase: the run passes, but the base is 200+ commits behind or
changed the same files
+ running --> needs_project_ci: the run passes, and the pull request changes
CI-defining files
+ failed --> running: the author pushes a fix
+ rebase --> running: the author rebases and pushes
+ ready --> running: the author pushes
+ ready --> approved: a maintainer approves
+ rebase --> approved: a maintainer approves, the merge waits for the rebase
+ ready --> project_ci: a maintainer adds use project ci
+ needs_project_ci --> project_ci: a maintainer reviews the CI change and
adds use project ci
+ project_ci --> approved: project CI passes and a maintainer approves
Review Comment:
Currently, even after maintainers add the `use project ci` label, PR will
not run project CI until the next push (or reopen), and it also needs
maintainers to "approve workflow".
Maybe we should make it more intuitive?
##########
dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md:
##########
@@ -109,6 +109,58 @@ Four pieces, in dependency order:
Plus `breeze ci audit` for the contributor's local setup, and documentation.
+### Pull request lifecycle
+
+What a gated pull request shows, and who acts next, from the moment it is
opened until it is merged
+or closed. The [state machine](#state-machine) below is the reconciler's view
of the same flow; this
+diagram is the contributor's and the maintainer's view. Pull requests of
committers, collaborators
+and exempted organisations are not gated and follow the usual project CI flow.
+
+```mermaid
+%%{init: {'theme': 'neutral'}}%%
+stateDiagram-v2
+ state "Waiting for the author: draft, fork CI not set up or no run for the
head commit" as waiting
+ state "Fork CI running" as running
+ state "Fork CI failed: out of the review queue" as failed
+ state "Green: ready for maintainer review, CI passed" as ready
+ state "Green, base too old: ready for maintainer review, CI passed pending
until a rebase" as rebase
+ state "Green, CI files changed: needs project ci" as needs_project_ci
+ state "Project CI: use project ci" as project_ci
+ state "Approved" as approved
+ state "Merged" as merged
+ state "Closed after 30 days without fork CI activity" as closed
+
+ [*] --> waiting: an external contributor opens a pull request
+ waiting --> running: the author enables the fork workflow and pushes
+ running --> failed: a job fails, times out or is cancelled, or the run
cannot start
+ running --> ready: the run passes on a current base
+ running --> rebase: the run passes, but the base is 200+ commits behind or
changed the same files
+ running --> needs_project_ci: the run passes, and the pull request changes
CI-defining files
+ failed --> running: the author pushes a fix
+ rebase --> running: the author rebases and pushes
+ ready --> running: the author pushes
+ ready --> approved: a maintainer approves
+ rebase --> approved: a maintainer approves, the merge waits for the rebase
+ ready --> project_ci: a maintainer adds use project ci
+ needs_project_ci --> project_ci: a maintainer reviews the CI change and
adds use project ci
+ project_ci --> approved: project CI passes and a maintainer approves
+ approved --> merged: merged, or auto-merged once CI passed is green
+ waiting --> closed: 30 days without fork CI activity
+ closed --> waiting: the author reopens it
+ merged --> [*]
+```
+
+- **Draft** is used only while the pull request waits for its author to set up
fork CI or push. A
Review Comment:
If the author opened the pull request as a draft or converted it, it stays a
draft when fork CI passes, and it gets no `ready for maintainer review`. The
author acts next, as the `green, author's draft` row in the state machine table
says.
Would it make sense to say this here, and maybe add a "Green, author's
draft" state to the diagram?
##########
dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md:
##########
@@ -109,6 +109,58 @@ Four pieces, in dependency order:
Plus `breeze ci audit` for the contributor's local setup, and documentation.
+### Pull request lifecycle
+
+What a gated pull request shows, and who acts next, from the moment it is
opened until it is merged
+or closed. The [state machine](#state-machine) below is the reconciler's view
of the same flow; this
+diagram is the contributor's and the maintainer's view. Pull requests of
committers, collaborators
+and exempted organisations are not gated and follow the usual project CI flow.
+
+```mermaid
+%%{init: {'theme': 'neutral'}}%%
+stateDiagram-v2
+ state "Waiting for the author: draft, fork CI not set up or no run for the
head commit" as waiting
+ state "Fork CI running" as running
+ state "Fork CI failed: out of the review queue" as failed
+ state "Green: ready for maintainer review, CI passed" as ready
+ state "Green, base too old: ready for maintainer review, CI passed pending
until a rebase" as rebase
+ state "Green, CI files changed: needs project ci" as needs_project_ci
+ state "Project CI: use project ci" as project_ci
+ state "Approved" as approved
+ state "Merged" as merged
+ state "Closed after 30 days without fork CI activity" as closed
+
+ [*] --> waiting: an external contributor opens a pull request
+ waiting --> running: the author enables the fork workflow and pushes
+ running --> failed: a job fails, times out or is cancelled, or the run
cannot start
+ running --> ready: the run passes on a current base
+ running --> rebase: the run passes, but the base is 200+ commits behind or
changed the same files
+ running --> needs_project_ci: the run passes, and the pull request changes
CI-defining files
+ failed --> running: the author pushes a fix
+ rebase --> running: the author rebases and pushes
+ ready --> running: the author pushes
+ ready --> approved: a maintainer approves
+ rebase --> approved: a maintainer approves, the merge waits for the rebase
+ ready --> project_ci: a maintainer adds use project ci
+ needs_project_ci --> project_ci: a maintainer reviews the CI change and
adds use project ci
+ project_ci --> approved: project CI passes and a maintainer approves
+ approved --> merged: merged, or auto-merged once CI passed is green
Review Comment:
`Approved` has no way back to `Fork CI running`, so the "merge waits for the
rebase" path from line 143 cannot finish in the diagram.
Maybe add `approved --> running: the author pushes or rebases`. I rendered
it with mermaid-cli and it lays out fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
