You've already forked AllSpice-Trial-Repo-Template
1017 lines
41 KiB
YAML
1017 lines
41 KiB
YAML
name: Export Design Review Comments
|
|
|
|
on:
|
|
# Run whenever a Design Review is closed
|
|
pull_request:
|
|
types: [closed]
|
|
# Allows the action to be triggered manually with a Design Review number
|
|
workflow_dispatch:
|
|
inputs:
|
|
dr_number:
|
|
description: "Design review number"
|
|
required: true
|
|
|
|
jobs:
|
|
export-design-review-comments:
|
|
name: Export Design Review Comments
|
|
# Merged design reviews only. Closing a design review without merging it
|
|
# does not run this workflow.
|
|
if: allspice.event_name == 'workflow_dispatch' || allspice.event.pull_request.merged
|
|
runs-on: ubuntu-latest
|
|
env:
|
|
DR_NUMBER: ${{ allspice.event.inputs.dr_number || allspice.event.pull_request.number }}
|
|
OUTPUT_FILE: design-review-comments.xlsx
|
|
steps:
|
|
- name: Set up Python
|
|
uses: actions/setup-python@v5
|
|
with:
|
|
python-version: "3.12"
|
|
|
|
- name: Install dependencies
|
|
run: pip install py-allspice openpyxl pillow cairosvg strip-markdown
|
|
|
|
- name: Export
|
|
env:
|
|
ALLSPICE_HUB_URL: ${{ allspice.server_url }}
|
|
ALLSPICE_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
|
REPO: ${{ allspice.repository }}
|
|
run: |
|
|
python - <<'PY'
|
|
import io
|
|
import os
|
|
import re
|
|
import time
|
|
import zipfile
|
|
from dataclasses import dataclass, field
|
|
from urllib.parse import urljoin, urlparse
|
|
from xml.etree import ElementTree
|
|
|
|
import cairosvg
|
|
import requests
|
|
import strip_markdown
|
|
from allspice import AllSpice, DesignReview, Repository
|
|
from allspice.exceptions import NotFoundException, NotYetGeneratedException
|
|
from openpyxl import Workbook
|
|
from openpyxl.cell.rich_text import CellRichText, TextBlock
|
|
from openpyxl.cell.text import InlineFont
|
|
from openpyxl.drawing.image import Image
|
|
from openpyxl.styles import Alignment, Border, Font, Side
|
|
from openpyxl.worksheet.hyperlink import Hyperlink
|
|
|
|
# The form's columns. A field's label is right aligned across B and C, its value
|
|
# goes in D. The findings table spans C to H.
|
|
LABEL_COLUMN = 2
|
|
FINDINGS_FIRST_COLUMN = 3
|
|
VALUE_COLUMN = 4
|
|
|
|
FINDING_COLUMNS = [
|
|
"REFERENCE",
|
|
"REVIEW COMMENT",
|
|
"REVIEWER",
|
|
"RESPONSIBLE",
|
|
"ACTION TAKEN",
|
|
"Open / Resolved",
|
|
]
|
|
|
|
LABEL_FONT = Font(name="Calibri", bold=True, size=12)
|
|
FIELD_FONT = Font(name="Arial", bold=True, size=12)
|
|
HEADING_FONT = Font(name="Arial", bold=True, size=10)
|
|
BODY_FONT = Font(name="Arial", size=10)
|
|
# Dresses only the words that name a picture, so the rest of a comment
|
|
# stays plain. A hyperlink always covers its whole cell, so this blue
|
|
# underline is what shows a reader where the link is.
|
|
LINK_FONT = InlineFont(rFont="Arial", sz=10, color="FF0000FF", u="single")
|
|
|
|
MEDIUM = Side(style="medium")
|
|
THIN = Side(style="thin")
|
|
BOX = Border(top=MEDIUM, bottom=MEDIUM, left=MEDIUM, right=MEDIUM)
|
|
|
|
RIGHT = Alignment(horizontal="right", vertical="center")
|
|
LEFT = Alignment(horizontal="left", vertical="center", wrap_text=True)
|
|
CENTRED = Alignment(horizontal="center", vertical="center", wrap_text=True)
|
|
TOP = Alignment(vertical="top", wrap_text=True)
|
|
|
|
FIELD_ROW_HEIGHT = 17
|
|
|
|
hub_url = os.environ["ALLSPICE_HUB_URL"].rstrip("/")
|
|
allspice = AllSpice(
|
|
allspice_hub_url=hub_url,
|
|
token_text=os.environ["ALLSPICE_TOKEN"],
|
|
)
|
|
|
|
owner, repo_name = os.environ["REPO"].split("/")
|
|
repository = Repository.request(allspice, owner, repo_name)
|
|
dr = DesignReview.request(allspice, owner, repo_name, os.environ["DR_NUMBER"])
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Reading the design review
|
|
#
|
|
# Everything here reads from the Hub and reduces what it finds to the rows the
|
|
# form wants. Pictures are numbered as they are met so that a comment can name
|
|
# them, but nothing is drawn until later.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@dataclass(frozen=True)
|
|
class Region:
|
|
"""A rectangle of a page that somebody selected while commenting.
|
|
|
|
The rectangle is held as fractions of one page of the sheet, left, top,
|
|
right and bottom, which is how the Hub records it. The page is named by
|
|
the directive's doc-id.
|
|
"""
|
|
|
|
path: str
|
|
commit: str
|
|
page: str
|
|
coords: tuple
|
|
aspect_ratio: float
|
|
|
|
|
|
@dataclass
|
|
class Finding:
|
|
"""One row of a findings table."""
|
|
|
|
reference: str
|
|
comment: str
|
|
reviewer: str
|
|
action_taken: str
|
|
state: str
|
|
pictures: list = field(default_factory=list)
|
|
|
|
|
|
@dataclass
|
|
class Section:
|
|
"""One review's part of the form: its heading and its findings."""
|
|
|
|
date: str
|
|
description: str
|
|
participants: list
|
|
findings: list
|
|
|
|
|
|
def display_name(user):
|
|
"""Name a person as they appear in the Hub, by login only if unnamed."""
|
|
return user.full_name or user.username
|
|
|
|
|
|
def author_name(entity):
|
|
"""Name whoever wrote a review or comment.
|
|
|
|
A review copied in from another Hub keeps its writer's name only in
|
|
original_author — its local user is whoever performed the copy. DRCY
|
|
posts through the actions user, so that user is given the name a
|
|
reader knows it by, whatever its own display name says.
|
|
"""
|
|
original = getattr(entity, "original_author", None)
|
|
if original:
|
|
return "DRCY" if original == "actions-bot" else original
|
|
if entity.user.username == "actions-bot":
|
|
return "DRCY"
|
|
return display_name(entity.user)
|
|
|
|
|
|
# A thumbnail directive is how a comment carries a picture of part of a sheet.
|
|
# The Hub draws it in place of the directive, so the text of the comment reads as
|
|
# if the directive were not there, and this export does the same.
|
|
SNIPPET_PATTERN = re.compile(r"!(?:snippet|thumbnail)\[[^\]]*\]\(([^)]*)\)\s*\{([^}]*)\}")
|
|
ATTRIBUTE_PATTERN = re.compile(r'([\w-]+)\s*=\s*(?:"([^"]*)"|(\S+))')
|
|
|
|
|
|
def plain_text(body):
|
|
"""Reduce a markdown comment to text, which reads better in a cell."""
|
|
return strip_markdown.strip_markdown(SNIPPET_PATTERN.sub("", body or "")).strip()
|
|
|
|
|
|
def regions_in(body):
|
|
"""Every region of a sheet that a comment points at.
|
|
|
|
A directive names the sheet, the diff it was read from and the rectangle that
|
|
was selected. Anything missing one of those is passed over, since it cannot
|
|
be drawn without all three.
|
|
"""
|
|
regions = []
|
|
for match in SNIPPET_PATTERN.finditer(body or ""):
|
|
# The path is escaped when written into markdown, to protect brackets.
|
|
path = re.sub(r"\\([()])", r"\1", match.group(1)).strip()
|
|
attributes = {
|
|
name: quoted or bare
|
|
for name, quoted, bare in ATTRIBUTE_PATTERN.findall(match.group(2))
|
|
}
|
|
|
|
# The commit is the head side of the diff the comment was left on.
|
|
_, _, revisions = attributes.get("diff", "").partition(":")
|
|
_, _, commit = revisions.partition("...")
|
|
|
|
parts = attributes.get("view-coords", "").split(",")
|
|
if not path or not commit or len(parts) != 4:
|
|
continue
|
|
try:
|
|
coords = tuple(float(part) / 100 for part in parts)
|
|
except ValueError:
|
|
continue
|
|
|
|
try:
|
|
aspect_ratio = float(attributes.get("aspect-ratio", 0))
|
|
except ValueError:
|
|
aspect_ratio = 0
|
|
|
|
regions.append(Region(
|
|
path=path,
|
|
commit=commit,
|
|
page=attributes.get("doc-id", ""),
|
|
coords=coords,
|
|
aspect_ratio=aspect_ratio,
|
|
))
|
|
return regions
|
|
|
|
|
|
IMAGE_PATTERN = re.compile(
|
|
r"!\[[^\]]*\]\(\s*<?([^)>\s]+)|<img[^>]+src=[\"']([^\"']+)[\"']",
|
|
re.IGNORECASE,
|
|
)
|
|
IMAGE_SUFFIXES = (".png", ".jpg", ".jpeg", ".gif", ".bmp", ".webp")
|
|
|
|
|
|
def image_urls_for(comment):
|
|
"""Every picture pasted into a comment or attached to it.
|
|
|
|
A plain link is not a picture and is left alone. Addresses are made absolute
|
|
so that the same attachment written in by hand and attached is recognised as
|
|
one picture rather than two.
|
|
"""
|
|
written_in = [
|
|
match.group(1) or match.group(2)
|
|
for match in IMAGE_PATTERN.finditer(comment.body or "")
|
|
]
|
|
|
|
# Attachments can be turned off for a whole Hub, which leaves this route
|
|
# missing rather than empty.
|
|
try:
|
|
assets = allspice.requests_get(
|
|
f"/repos/{owner}/{repo_name}/issues/comments/{comment.id}/assets"
|
|
) or []
|
|
except NotFoundException:
|
|
assets = []
|
|
|
|
attached = [
|
|
asset["browser_download_url"]
|
|
for asset in assets
|
|
if asset["name"].lower().endswith(IMAGE_SUFFIXES)
|
|
]
|
|
|
|
absolute = [urljoin(f"{hub_url}/", url) for url in written_in + attached]
|
|
return list(dict.fromkeys(absolute))
|
|
|
|
|
|
# Pictures are numbered in the order a reader meets them, going down the design
|
|
# review, so that a comment can name its pictures before any have been drawn.
|
|
# Each one is remembered under the row it will be drawn on.
|
|
picture_numbers = {}
|
|
regions_by_sheet = {}
|
|
urls_by_number = {}
|
|
|
|
|
|
def number_picture(key):
|
|
"""Give a picture its number, and say whether this is the first sighting."""
|
|
if key in picture_numbers:
|
|
return picture_numbers[key], False
|
|
picture_numbers[key] = len(picture_numbers) + 1
|
|
return picture_numbers[key], True
|
|
|
|
|
|
def pictures_for(comment):
|
|
"""Number every picture a comment shows, and queue it to be drawn.
|
|
|
|
Regions are queued by the sheet they came from, so that each sheet is only
|
|
read once no matter how many comments point into it.
|
|
"""
|
|
numbers = []
|
|
|
|
for region in regions_in(comment.body):
|
|
number, first_sighting = number_picture(region)
|
|
if first_sighting:
|
|
sheet = (region.path, region.commit)
|
|
regions_by_sheet.setdefault(sheet, []).append((number, region))
|
|
numbers.append(number)
|
|
|
|
for url in image_urls_for(comment):
|
|
number, first_sighting = number_picture(url)
|
|
if first_sighting:
|
|
urls_by_number[number] = url
|
|
numbers.append(number)
|
|
|
|
return numbers
|
|
|
|
|
|
def is_unhelpful(comment):
|
|
"""Whether a comment has been marked as not worth keeping.
|
|
|
|
A thumbs down is how a reviewer says a comment wasn't useful. The reactions
|
|
route answers null rather than an empty list when a comment has none.
|
|
"""
|
|
reactions = allspice.requests_get(
|
|
f"/repos/{owner}/{repo_name}/issues/comments/{comment.id}/reactions"
|
|
) or []
|
|
return any(reaction["content"] == "-1" for reaction in reactions)
|
|
|
|
|
|
def comment_location(comment):
|
|
"""Where in the design a comment was left.
|
|
|
|
A comment's line arrives split across two fields: original_position holds it
|
|
for the left side of the diff and position for the right, with the unused one
|
|
left at zero. Both are needed to tell a comment on one side of a file apart
|
|
from one on the other.
|
|
"""
|
|
if comment.original_position:
|
|
return (comment.path, comment.sub_path, "left", comment.original_position)
|
|
return (comment.path, comment.sub_path, "right", comment.position)
|
|
|
|
|
|
def conversations(review):
|
|
"""A review's comments, grouped into the conversations they form."""
|
|
if not review.comments_count:
|
|
return []
|
|
|
|
grouped = {}
|
|
for comment in review.get_comments():
|
|
grouped.setdefault(comment_location(comment), []).append(comment)
|
|
|
|
in_order = [
|
|
sorted(chain, key=lambda comment: (comment.created_at, comment.id))
|
|
for chain in grouped.values()
|
|
]
|
|
return sorted(in_order, key=lambda chain: (chain[0].created_at, chain[0].id))
|
|
|
|
|
|
def reference_for(comment):
|
|
"""Name where a comment sits: its file, and the element within it when
|
|
the comment was left on one rather than on the file as a whole."""
|
|
file_name = os.path.basename(comment.path)
|
|
if comment.sub_path:
|
|
return f"{file_name}:{comment.sub_path}"
|
|
return file_name
|
|
|
|
|
|
def read_finding(conversation):
|
|
"""Reduce a conversation to the one row it contributes.
|
|
|
|
The opening comment is the review comment, and the design review author's
|
|
last word in the thread is the action taken. Whatever was said in between is
|
|
working discussion, which is deliberately not carried into the record.
|
|
"""
|
|
opening = conversation[0]
|
|
answers = [
|
|
comment for comment in conversation[1:] if comment.user.id == dr.user.id
|
|
]
|
|
|
|
return Finding(
|
|
reference=reference_for(opening),
|
|
comment=plain_text(opening.body),
|
|
reviewer=author_name(opening),
|
|
action_taken=plain_text(answers[-1].body) if answers else "",
|
|
# Resolving a conversation marks its opening comment alone.
|
|
state="Resolved" if opening.resolver else "Open",
|
|
pictures=pictures_for(opening),
|
|
)
|
|
|
|
|
|
def read_section(review):
|
|
"""Reduce a review to its part of the form, or None when it is empty.
|
|
|
|
A review that said nothing and left no comments is a by-product of submitting
|
|
one without writing anything, so it earns no section.
|
|
"""
|
|
kept = [
|
|
conversation
|
|
for conversation in conversations(review)
|
|
if not is_unhelpful(conversation[0])
|
|
]
|
|
description = plain_text(review.body)
|
|
|
|
print(
|
|
f"Review {review.id} by {author_name(review)}: "
|
|
f"{len(kept)} conversations"
|
|
)
|
|
|
|
if not description and not kept:
|
|
return None
|
|
|
|
# Everyone who took part, whether they opened a conversation or answered one.
|
|
participants = {author_name(review)}
|
|
for conversation in kept:
|
|
for comment in conversation:
|
|
participants.add(author_name(comment))
|
|
|
|
return Section(
|
|
date=(review.submitted_at or "")[:10],
|
|
description=description,
|
|
participants=sorted(participants),
|
|
findings=[read_finding(conversation) for conversation in kept],
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Drawing the pictures
|
|
#
|
|
# A sheet is read once, every region of it is drawn, and then it is let go, so
|
|
# only one sheet is held at a time however many regions there are.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
REFERENCE_ROW_HEIGHT = 409.5
|
|
# A picture is scaled to fit inside these bounds rather than to fill them, so a
|
|
# wide one shrinks instead of running across the columns beside it. The row has
|
|
# to stay taller than MAX_HEIGHT for the picture to fit within it.
|
|
MAX_HEIGHT = 520
|
|
MAX_WIDTH = 460
|
|
# Drawn wider than it is shown, so it stays sharp when a reader zooms in.
|
|
DRAWN_WIDTH = 920
|
|
|
|
PICTURE_FAILURES = (
|
|
requests.RequestException,
|
|
NotFoundException,
|
|
NotYetGeneratedException,
|
|
ElementTree.ParseError,
|
|
OSError,
|
|
ValueError,
|
|
)
|
|
|
|
|
|
def fetch(address):
|
|
"""Read an address, sending the Hub's token only to the Hub itself.
|
|
|
|
Anyone who can comment can put a link to anywhere into one, so the token is
|
|
withheld from every address that is not on this Hub.
|
|
"""
|
|
on_hub = urlparse(address).hostname == urlparse(hub_url).hostname
|
|
response = allspice.requests.get(
|
|
address, headers=allspice.headers if on_hub else None
|
|
)
|
|
response.raise_for_status()
|
|
return response.content
|
|
|
|
|
|
def read_sheet(path, commit):
|
|
"""Ask the Hub for a drawing, as an SVG.
|
|
|
|
A drawing that has not been rendered before is queued and answered later, so
|
|
this waits for it rather than giving up on the first ask.
|
|
"""
|
|
ATTEMPTS = 12
|
|
WAIT = 5
|
|
|
|
for attempt in range(ATTEMPTS):
|
|
try:
|
|
return repository.get_generated_svg(path, ref=commit)
|
|
except NotYetGeneratedException:
|
|
if attempt == ATTEMPTS - 1:
|
|
raise
|
|
time.sleep(WAIT)
|
|
|
|
|
|
def draw_region(svg, region):
|
|
"""Draw one region of a sheet as a picture.
|
|
|
|
The fractions in a comment were measured against one page of the drawing,
|
|
never the drawing as a whole. Each page is a group inside the SVG, placed
|
|
on the drawing by a transform and carrying its own rectangle in its
|
|
data-view-box attribute. So the page is cut out the way the Hub shows one:
|
|
keep its group alone, take off its transform, and narrow the view to the
|
|
region's fractions of the page's own rectangle.
|
|
"""
|
|
ElementTree.register_namespace("", "http://www.w3.org/2000/svg")
|
|
root = ElementTree.fromstring(svg)
|
|
|
|
pages = [
|
|
group
|
|
for group in root.findall("{http://www.w3.org/2000/svg}g")
|
|
if group.get("data-view-box")
|
|
]
|
|
page = next(
|
|
(group for group in pages if group.get("data-document-id") == region.page),
|
|
None,
|
|
)
|
|
if page is None and len(pages) == 1:
|
|
page = pages[0]
|
|
|
|
if page is None:
|
|
# With no page to go by, the whole drawing is what is left. The shape
|
|
# check below says when that was the wrong call.
|
|
window = root.get("viewBox", "")
|
|
else:
|
|
for other in pages:
|
|
if other is not page:
|
|
root.remove(other)
|
|
page.attrib.pop("transform", None)
|
|
window = page.get("data-view-box")
|
|
|
|
window = [float(number) for number in window.replace(",", " ").split()]
|
|
if len(window) != 4:
|
|
raise ValueError(f"{region.path} has no viewBox to narrow")
|
|
min_x, min_y, width, height = window
|
|
|
|
# The comment recorded the shape of the page it was measured against. A page
|
|
# of a different shape means the wrong one was picked, which would put the
|
|
# region somewhere else entirely.
|
|
if region.aspect_ratio and height:
|
|
drawn = width / height
|
|
if abs(drawn - region.aspect_ratio) > 0.05 * region.aspect_ratio:
|
|
print(
|
|
f" {region.path} page {region.page or '(only page)'} is {drawn:.2f} "
|
|
f"wide for its height, but the comment measured {region.aspect_ratio:.2f}"
|
|
)
|
|
|
|
left, top, right, bottom = region.coords
|
|
root.set("viewBox", " ".join(f"{value:.3f}" for value in (
|
|
min_x + left * width,
|
|
min_y + top * height,
|
|
(right - left) * width,
|
|
(bottom - top) * height,
|
|
)))
|
|
root.set("width", "100%")
|
|
root.set("height", "100%")
|
|
|
|
return cairosvg.svg2png(
|
|
bytestring=ElementTree.tostring(root), output_width=DRAWN_WIDTH
|
|
)
|
|
|
|
|
|
def place_picture(sheet, number, content):
|
|
"""Put a picture on the References sheet, on the row it was numbered for."""
|
|
row = number + 1
|
|
picture = Image(io.BytesIO(content))
|
|
if not picture.width or not picture.height:
|
|
raise ValueError("picture has no size")
|
|
|
|
scale = min(MAX_HEIGHT / picture.height, MAX_WIDTH / picture.width)
|
|
picture.width = int(picture.width * scale)
|
|
picture.height = int(picture.height * scale)
|
|
|
|
styled(
|
|
sheet.cell(row=row, column=1, value=f"Reference {number}"),
|
|
BODY_FONT,
|
|
alignment=CENTRED,
|
|
)
|
|
sheet.add_image(picture, f"B{row}")
|
|
sheet.row_dimensions[row].height = REFERENCE_ROW_HEIGHT
|
|
|
|
|
|
def draw_references(sheet):
|
|
"""Draw every numbered picture onto the References sheet.
|
|
|
|
A picture that cannot be read is reported and left out. The rest of the
|
|
export carries on without it, since a missing picture is worth less than a
|
|
failed run.
|
|
"""
|
|
for (path, commit), queued in regions_by_sheet.items():
|
|
try:
|
|
svg = read_sheet(path, commit)
|
|
except PICTURE_FAILURES as error:
|
|
print(f"Could not read {path} at {commit[:8]}: {error}")
|
|
continue
|
|
|
|
print(
|
|
f"Read {path} at {commit[:8]}: "
|
|
f"{len(svg) // 1024}KB, {len(queued)} regions"
|
|
)
|
|
for number, region in queued:
|
|
try:
|
|
place_picture(sheet, number, draw_region(svg, region))
|
|
except PICTURE_FAILURES as error:
|
|
print(f"Could not draw Reference {number} of {path}: {error}")
|
|
svg = None
|
|
|
|
for number, url in urls_by_number.items():
|
|
try:
|
|
place_picture(sheet, number, fetch(url))
|
|
except PICTURE_FAILURES as error:
|
|
print(f"Could not read a picture for Reference {number} from {url}: {error}")
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Building the spreadsheet
|
|
#
|
|
# Everything here writes cells. A function that fills a run of rows takes the row
|
|
# it starts on and gives back the row after the last one it wrote, so the callers
|
|
# below never count rows themselves.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def styled(cell, font, border=None, alignment=None):
|
|
"""Give a cell whichever of its looks are supplied."""
|
|
cell.font = font
|
|
if border:
|
|
cell.border = border
|
|
if alignment:
|
|
cell.alignment = alignment
|
|
return cell
|
|
|
|
|
|
def span(sheet, row, first_column, last_column, font=None, border=None):
|
|
"""Merge a run of cells, styling each one.
|
|
|
|
A border set on a merged range only reaches its first cell, so every cell
|
|
in the run needs it for the range to look whole.
|
|
"""
|
|
sheet.merge_cells(
|
|
start_row=row,
|
|
start_column=first_column,
|
|
end_row=row,
|
|
end_column=last_column,
|
|
)
|
|
for column in range(first_column, last_column + 1):
|
|
styled(sheet.cell(row=row, column=column), font or BODY_FONT, border)
|
|
|
|
|
|
def write_blank_rows(sheet, row, count):
|
|
"""Leave rows empty, merged across the label columns.
|
|
|
|
An unmerged blank row draws a line down the middle of the form, so even a row
|
|
with nothing in it is merged.
|
|
"""
|
|
for row_number in range(row, row + count):
|
|
span(sheet, row_number, LABEL_COLUMN, FINDINGS_FIRST_COLUMN)
|
|
return row + count
|
|
|
|
|
|
def write_field(sheet, row, label, value=""):
|
|
"""A labelled field: the label right aligned, the value boxed beside it."""
|
|
span(sheet, row, LABEL_COLUMN, FINDINGS_FIRST_COLUMN, LABEL_FONT)
|
|
styled(
|
|
sheet.cell(row=row, column=LABEL_COLUMN, value=label),
|
|
LABEL_FONT,
|
|
alignment=RIGHT,
|
|
)
|
|
styled(
|
|
sheet.cell(row=row, column=VALUE_COLUMN, value=value),
|
|
FIELD_FONT,
|
|
border=BOX,
|
|
alignment=LEFT,
|
|
)
|
|
sheet.row_dimensions[row].height = FIELD_ROW_HEIGHT
|
|
return row + 1
|
|
|
|
|
|
def finding_cells(finding):
|
|
"""A finding's cells, and the reference its pictures start at.
|
|
|
|
A picture cannot be held in a cell, so each one is drawn on the References
|
|
sheet and named in the comment instead. Only the naming words are dressed
|
|
as a link, so the comment itself stays plain text.
|
|
"""
|
|
comment = finding.comment
|
|
if finding.pictures:
|
|
named = ", ".join(f"Reference {number}" for number in finding.pictures)
|
|
lead = f"{comment}\n(See " if comment else "(See "
|
|
comment = CellRichText(lead, TextBlock(LINK_FONT, named), ")")
|
|
|
|
cells = [
|
|
finding.reference,
|
|
comment,
|
|
finding.reviewer,
|
|
# The design review's author owns their own design, so they are
|
|
# responsible for acting on every comment left on it.
|
|
display_name(dr.user),
|
|
finding.action_taken,
|
|
finding.state,
|
|
]
|
|
return cells, finding.pictures[0] if finding.pictures else None
|
|
|
|
|
|
def write_findings_table(sheet, row, findings):
|
|
"""A section's findings, with spare rows left to be added to by hand."""
|
|
SPACER_ROW_HEIGHT = 5.25
|
|
ROW_HEIGHT = 16
|
|
MINIMUM_ROWS = 5
|
|
HEADING_BOX = Border(top=MEDIUM, bottom=MEDIUM, left=THIN, right=THIN)
|
|
|
|
row = write_blank_rows(sheet, row, 1)
|
|
sheet.row_dimensions[row - 1].height = SPACER_ROW_HEIGHT
|
|
|
|
# The headings are written even when a review raised nothing, so that every
|
|
# section reads the same way.
|
|
styled(
|
|
sheet.cell(row=row, column=LABEL_COLUMN),
|
|
HEADING_FONT,
|
|
Border(top=MEDIUM, bottom=MEDIUM, right=THIN),
|
|
CENTRED,
|
|
)
|
|
for offset, heading in enumerate(FINDING_COLUMNS):
|
|
styled(
|
|
sheet.cell(row=row, column=FINDINGS_FIRST_COLUMN + offset, value=heading),
|
|
HEADING_FONT,
|
|
HEADING_BOX,
|
|
CENTRED,
|
|
)
|
|
sheet.row_dimensions[row].height = ROW_HEIGHT
|
|
row += 1
|
|
|
|
total_rows = max(len(findings), MINIMUM_ROWS)
|
|
for index in range(total_rows):
|
|
finding = findings[index] if index < len(findings) else None
|
|
|
|
# The two sides of a shared edge are given the same weight. A spreadsheet
|
|
# reader is free to draw either one of them, so they have to agree for the
|
|
# table's outline to be certain.
|
|
first = index == 0
|
|
last = index == total_rows - 1
|
|
|
|
styled(
|
|
sheet.cell(row=row, column=LABEL_COLUMN),
|
|
BODY_FONT,
|
|
border=Border(
|
|
top=MEDIUM if first else None,
|
|
bottom=MEDIUM if last else None,
|
|
),
|
|
)
|
|
|
|
cells = [""] * len(FINDING_COLUMNS)
|
|
reference = None
|
|
if finding:
|
|
cells, reference = finding_cells(finding)
|
|
|
|
for offset, value in enumerate(cells):
|
|
styled(
|
|
sheet.cell(row=row, column=FINDINGS_FIRST_COLUMN + offset, value=value),
|
|
BODY_FONT,
|
|
border=Border(
|
|
top=MEDIUM if first else THIN,
|
|
bottom=MEDIUM if last else THIN,
|
|
left=THIN,
|
|
right=THIN,
|
|
),
|
|
alignment=TOP,
|
|
)
|
|
|
|
# A comment carrying pictures links through to the first of them.
|
|
if reference:
|
|
linked = sheet.cell(row=row, column=FINDINGS_FIRST_COLUMN + 1)
|
|
linked.hyperlink = Hyperlink(
|
|
ref=linked.coordinate,
|
|
location=f"References!B{reference + 1}",
|
|
)
|
|
|
|
# A spare row is a fixed height. A row holding a comment is left to size
|
|
# itself around the text.
|
|
if finding is None:
|
|
sheet.row_dimensions[row].height = ROW_HEIGHT
|
|
row += 1
|
|
|
|
return row
|
|
|
|
|
|
def write_form_heading(sheet):
|
|
"""The top of the form, above the first review's section."""
|
|
BANNER_LAST_COLUMN = 6
|
|
TITLE_FONT = Font(name="Calibri", bold=True, size=16)
|
|
NUMBER_FONT = Font(name="Arial", bold=True, italic=True)
|
|
COLUMN_WIDTHS = {
|
|
"A": 1, "B": 12, "C": 42, "D": 62.33,
|
|
"E": 40.16, "F": 24.66, "G": 69.16, "H": 15.5,
|
|
}
|
|
ROW_HEIGHTS = ((1, 8.25), (2, 22), (3, 9), (4, 16), (5, 5.25))
|
|
|
|
for column, width in COLUMN_WIDTHS.items():
|
|
sheet.column_dimensions[column].width = width
|
|
for row, height in ROW_HEIGHTS:
|
|
sheet.row_dimensions[row].height = height
|
|
|
|
# The banner across the top.
|
|
for column in range(LABEL_COLUMN, BANNER_LAST_COLUMN + 1):
|
|
styled(
|
|
sheet.cell(row=2, column=column),
|
|
TITLE_FONT if column == LABEL_COLUMN else BODY_FONT,
|
|
Border(
|
|
top=MEDIUM,
|
|
bottom=MEDIUM,
|
|
left=MEDIUM if column == LABEL_COLUMN else None,
|
|
right=MEDIUM if column == BANNER_LAST_COLUMN else None,
|
|
),
|
|
)
|
|
sheet.cell(row=2, column=LABEL_COLUMN, value="DESIGN REVIEW REPORT")
|
|
|
|
# The design review's number, so the file says which review it records.
|
|
span(sheet, 4, LABEL_COLUMN, FINDINGS_FIRST_COLUMN, HEADING_FONT, BOX)
|
|
styled(
|
|
sheet.cell(row=4, column=VALUE_COLUMN, value=f"DR #{dr.number}"),
|
|
NUMBER_FONT,
|
|
border=BOX,
|
|
alignment=CENTRED,
|
|
)
|
|
span(
|
|
sheet, 4, VALUE_COLUMN + 1, BANNER_LAST_COLUMN,
|
|
HEADING_FONT, BOX,
|
|
)
|
|
write_blank_rows(sheet, 3, 1)
|
|
write_blank_rows(sheet, 5, 1)
|
|
|
|
row = write_field(sheet, 6, "Title:", dr.title)
|
|
return write_blank_rows(sheet, row, 1)
|
|
|
|
|
|
def write_section(sheet, row, section):
|
|
"""One review's heading and findings table."""
|
|
row = write_field(sheet, row, "Date:", section.date)
|
|
row = write_field(sheet, row, "Description of Review:", section.description)
|
|
row = write_field(sheet, row, "Author:", display_name(dr.user))
|
|
|
|
for number, participant in enumerate(section.participants, start=1):
|
|
row = write_field(sheet, row, f"Reviewer {number}", participant)
|
|
|
|
return write_findings_table(sheet, row, section.findings)
|
|
|
|
|
|
def write_references_heading(sheet):
|
|
"""The sheet each of a comment's pictures is drawn on."""
|
|
sheet["A1"] = "Reference #"
|
|
sheet["B1"] = "Details"
|
|
sheet.column_dimensions["A"].width = 14
|
|
sheet.column_dimensions["B"].width = 67.16
|
|
|
|
|
|
def draw_left_edge(sheet, last_row):
|
|
"""Edge the form down its left side.
|
|
|
|
The edge is one unbroken line, so it is drawn in a single pass over every row
|
|
rather than left to each of them to remember.
|
|
"""
|
|
for row in range(3, last_row + 1):
|
|
cell = sheet.cell(row=row, column=LABEL_COLUMN)
|
|
cell.border = Border(
|
|
top=cell.border.top,
|
|
bottom=cell.border.bottom,
|
|
left=MEDIUM,
|
|
right=cell.border.right,
|
|
)
|
|
|
|
|
|
def share_rich_strings(path):
|
|
"""Move rich text out of the sheets and into the shared-strings table.
|
|
|
|
openpyxl stores every string inside its own cell, a form Excel itself
|
|
never writes. Other spreadsheet readers barely test that path: Numbers
|
|
drops the formatting and Google Sheets drops runs of the text itself.
|
|
So rich strings are moved to the shared-strings table, the form every
|
|
reader handles, and plain strings are left where they are.
|
|
"""
|
|
with zipfile.ZipFile(path) as archive:
|
|
parts = {name: archive.read(name) for name in archive.namelist()}
|
|
|
|
existing = parts.get("xl/sharedStrings.xml", b"").decode()
|
|
first = len(re.findall("<si>", existing))
|
|
strings = []
|
|
|
|
def take(match):
|
|
opening, runs = match.groups()
|
|
if "<r>" not in runs and "<r " not in runs:
|
|
return match.group(0)
|
|
strings.append(f"<si>{runs}</si>")
|
|
return f'{opening} t="s"><v>{first + len(strings) - 1}</v></c>'
|
|
|
|
for name, data in parts.items():
|
|
if re.fullmatch(r"xl/worksheets/sheet\d+[.]xml", name):
|
|
parts[name] = re.sub(
|
|
r'(<c [^>]*?) t="inlineStr"><is>(.*?)</is></c>',
|
|
take,
|
|
data.decode(),
|
|
flags=re.DOTALL,
|
|
).encode()
|
|
|
|
if not strings:
|
|
return
|
|
|
|
body = "".join(strings)
|
|
total = first + len(strings)
|
|
if existing:
|
|
parts["xl/sharedStrings.xml"] = re.sub(
|
|
r'count="\d+" uniqueCount="\d+"',
|
|
f'count="{total}" uniqueCount="{total}"',
|
|
existing.replace("</sst>", f"{body}</sst>"),
|
|
).encode()
|
|
else:
|
|
parts["xl/sharedStrings.xml"] = (
|
|
'<?xml version="1.0" encoding="UTF-8" standalone="yes"?>'
|
|
'<sst xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" '
|
|
f'count="{total}" uniqueCount="{total}">{body}</sst>'
|
|
).encode()
|
|
content_types = parts["[Content_Types].xml"].decode()
|
|
parts["[Content_Types].xml"] = content_types.replace(
|
|
"</Types>",
|
|
'<Override PartName="/xl/sharedStrings.xml" ContentType="application/'
|
|
'vnd.openxmlformats-officedocument.spreadsheetml.sharedStrings+xml"/>'
|
|
"</Types>",
|
|
).encode()
|
|
relationships = parts["xl/_rels/workbook.xml.rels"].decode()
|
|
free = max(int(n) for n in re.findall(r'Id="rId(\d+)"', relationships)) + 1
|
|
parts["xl/_rels/workbook.xml.rels"] = relationships.replace(
|
|
"</Relationships>",
|
|
f'<Relationship Id="rId{free}" '
|
|
'Type="http://schemas.openxmlformats.org/officeDocument/2006/'
|
|
'relationships/sharedStrings" Target="sharedStrings.xml"/>'
|
|
"</Relationships>",
|
|
).encode()
|
|
|
|
with zipfile.ZipFile(path, "w", zipfile.ZIP_DEFLATED) as archive:
|
|
for name, data in parts.items():
|
|
archive.writestr(name, data)
|
|
|
|
print(f"Moved {len(strings)} rich strings into the shared-strings table")
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Exporting
|
|
# ---------------------------------------------------------------------------
|
|
|
|
ROWS_BETWEEN_SECTIONS = 2
|
|
|
|
sections = [
|
|
section
|
|
for section in (read_section(review) for review in dr.get_reviews())
|
|
if section
|
|
]
|
|
|
|
# With nothing to export, no file is written and the remaining steps are skipped
|
|
# rather than publishing an empty spreadsheet.
|
|
if not sections:
|
|
print("No reviews to export.")
|
|
raise SystemExit(0)
|
|
|
|
workbook = Workbook()
|
|
review_info = workbook.active
|
|
review_info.title = "Review Info"
|
|
references = workbook.create_sheet("References")
|
|
|
|
write_references_heading(references)
|
|
|
|
row = write_form_heading(review_info)
|
|
for index, section in enumerate(sections):
|
|
if index:
|
|
row = write_blank_rows(review_info, row, ROWS_BETWEEN_SECTIONS)
|
|
row = write_section(review_info, row, section)
|
|
|
|
draw_left_edge(review_info, row - 1)
|
|
draw_references(references)
|
|
|
|
workbook.save(os.environ["OUTPUT_FILE"])
|
|
share_rich_strings(os.environ["OUTPUT_FILE"])
|
|
|
|
print(
|
|
f"Saved {os.environ['OUTPUT_FILE']} with {len(sections)} review sections "
|
|
f"and {len(picture_numbers)} pictures"
|
|
)
|
|
|
|
PY
|
|
|
|
# The official Github upload-artifact action refuses to run against anything that
|
|
# is not github.com, so this uses a fork that supports AllSpice Hub.
|
|
- name: Upload the export
|
|
if: hashFiles(env.OUTPUT_FILE) != ''
|
|
uses: https://github.com/ChristopherHX/gitea-upload-artifact@62ac910c5d3dfa85c7cb2df15afe2e342b2407c2
|
|
with:
|
|
name: design-review-${{ env.DR_NUMBER }}-comments
|
|
path: ${{ env.OUTPUT_FILE }}
|
|
|
|
- name: Comment on the design review
|
|
if: hashFiles(env.OUTPUT_FILE) != ''
|
|
env:
|
|
ALLSPICE_HUB_URL: ${{ allspice.server_url }}
|
|
ALLSPICE_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
|
REPO: ${{ allspice.repository }}
|
|
ARTIFACT_NAME: design-review-${{ env.DR_NUMBER }}-comments
|
|
RUN_NUMBER: ${{ allspice.run_number }}
|
|
run: |
|
|
python - <<'PY'
|
|
import os
|
|
|
|
from allspice import AllSpice, DesignReview
|
|
|
|
allspice = AllSpice(
|
|
allspice_hub_url=os.environ["ALLSPICE_HUB_URL"].rstrip("/"),
|
|
token_text=os.environ["ALLSPICE_TOKEN"],
|
|
)
|
|
|
|
owner, repo_name = os.environ["REPO"].split("/")
|
|
dr = DesignReview.request(
|
|
allspice, owner, repo_name, os.environ["DR_NUMBER"]
|
|
)
|
|
|
|
artifact_name = os.environ["ARTIFACT_NAME"]
|
|
|
|
# The design review's own URL is the most reliable base to build on: it
|
|
# carries the repository's real name and needs no joining by hand.
|
|
repository_url = dr.html_url.split("/pulls/")[0]
|
|
artifact_url = (
|
|
f"{repository_url}/actions/runs/{os.environ['RUN_NUMBER']}"
|
|
f"/artifacts/{artifact_name}"
|
|
)
|
|
|
|
dr.create_comment(
|
|
f"The comments on this design review have been exported to "
|
|
f"[{artifact_name}]({artifact_url})."
|
|
)
|
|
|
|
print(f"Commented on the design review with {artifact_url}")
|
|
PY
|