Use a Mermaid graph in the backmerge PR body #594
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,17 +110,58 @@ def self.create_backmerge_pr(token:, repository:, title:, head_branch:, base_bra | |
|
|
||
| other_action.push_to_git_remote(tags: false) | ||
|
|
||
| # Live playground to edit this graph | ||
| # https://mermaid.live/edit#pako:eNqNkU1PwzAMhv9KZanqpWVFwCVHQOKCxoFrLl7itdGaZEoTTWjqf8eLOo2i8eGTPx77TewjKK8JBJTlUbqCzTgTRTEHJ6s6E18C7vtqkc4li8Y9BnSqX6MlBqoYkttV9Y_cW9AUGLxt2_Y7Nfb-8OStNfEVNzQwtcVhpCU1XUJ2p7KU7vzAXNlkmSLQQDjS6v7m4S7nVU9q51O8UsmSv7jzSEuho9Xc3pzaG-Oib_KXlxr_QL8onbuuVy9unvrXbKiBCV645qvme0mIPVmSINjVtMU0RAm8O0YxRf_-4RQIbqca0l5jpGeDXUALIu9_-gRejKzU | ||
| meramid_git_graph = <<~GRAPH | ||
| ```mermaid | ||
| %%{ | ||
| init: { | ||
| 'gitGraph': { | ||
| 'mainBranchName': '#{base_branch}', | ||
| 'mainBranchOrder': 1000, | ||
| 'showCommitLabel': false | ||
| } | ||
| } | ||
| }%% | ||
| gitGraph | ||
| branch #{head_branch} | ||
| checkout #{head_branch} | ||
| commit | ||
| commit | ||
| commit | ||
| branch #{intermediate_branch} | ||
| checkout #{intermediate_branch} | ||
| commit | ||
| checkout #{base_branch} | ||
| commit | ||
| commit | ||
| merge #{base_branch} | ||
| ``` | ||
| GRAPH | ||
|
|
||
| ascii_git_graph = <<~GRAPH | ||
| ``` | ||
| #{head_branch.rjust(40)} ----o-- - - - | ||
| #{' ' * 40} \\ | ||
| #{intermediate_branch.rjust(40)} `---. | ||
| #{' ' * 40} \\ | ||
| #{base_branch.rjust(40)} ------------x- - - | ||
| ``` | ||
| GRAPH | ||
|
|
||
| pr_body = <<~BODY | ||
| Merging `#{head_branch}` into `#{base_branch}`. | ||
|
There was a problem hiding this comment. wdyt about adding the chart type as a parameter? Something like There was a problem hiding this comment. My first instinct was to say thatI'd rather always have both in case the Mermaid one fails, which is something we cannot know at the time we generated the body because it depends on GitHub. But, it's good to build flexible systems (unopinionated tools, opinionated golden path) and the adding a parameter like this does only makes the code a tiny bit more complex (simple is usually better). I'll update 👍 There was a problem hiding this comment.
Spawned #596 to not slow this down while waiting for that. |
||
|
|
||
| Via intermediate branch `#{intermediate_branch}`, to help fix conflicts if any: | ||
|
|
||
| #{meramid_git_graph} | ||
|
|
||
| <details> | ||
| <summary>Expand to see an ASCII representation of the Git graph above</summary> | ||
|
|
||
| #{ascii_git_graph} | ||
|
|
||
| </details> | ||
|
Comment on lines
+159
to
+164
There was a problem hiding this comment. Maybe we can mitigate the "disruption" of the graph not rendering correctly in GitHub for whichever reason by having the ASCII as a fallback here. |
||
| BODY | ||
|
|
||
| other_action.create_pull_request( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, at this time, there is no commit on the integration branch. However, without the commit, I don't think the Mermaid graph doesn't convey the idea of the integration branch as well:
without
commitwith
commitThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that was also a limitation I remember seeing when I tried it a while ago. It I think it makes sense and is ok to have that commit visually added there anyway.
Besides, this is not entirely true. For example, on Tumblr there is a commit done on the intermediate branch after it is cut from
release/—the goal of that extra commit being togit checkout $basebranch -- version.xcconfigto auto-resolve the conflict that would otherwise be present on that file due to it evolving separately onrelease/*branch (for daily betas) vsdevelop(for daily alphas).This is also why this Fastlane action has a
ConfigItemallowing you to pass aProcto add arbitrary commits on the intermediate branch after it's cut but before the PR is created, btw 😉There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO the fonts and the overall size of the chart look too big for me (or maybe I'm too used to the ASCII art 😄 ), taking a lot of space of the PR text block. Is there a way to make it a bit smaller?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also feels strange that the commits on
trunkseem to have happened only after the commits fromrelease/4.53in the screenshot above; It's like if the 3 commits fromtrunkthat were illustrated in this Mermaid graph were only pushed afterrelease/*andmerge/release-*-into-*had been created, instead of suggesting that thetrunkbranch has been existing and living its life for way longer beforerelease/*branch… 🤔