Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report improvements #68

Merged
merged 22 commits into from
Jun 27, 2022
Merged

Report improvements #68

merged 22 commits into from
Jun 27, 2022

Conversation

mds1
Copy link
Contributor

@mds1 mds1 commented Jun 8, 2022

Closes #33, replaces #67

Why we are closing #67 in favor of this approach:

  • The original plan was to update proposal checks to output generic results, independent of formatting, then have postprocessors to convert that to markdown, PDF, HTML, or whatever other format.
  • There's no standardized format I came across for doing this, so we were going to use a version of pdfmake's type definition as the format since it was high-level and seemed like a good option.
  • As we implemented the postprocessors, it became a decent amount of work with various edge cases, etc. In general it felt like we were re-inventing the wheel.
  • @wildmolasses pointed out that if we just output markdown, we can use tools like remark and md-to-pdf which let you convert markdown to an AST and convert to HTML/other formats from there, as well as edit the AST to modify content, etc
  • This means proposal checks are still expected to output their results as markdown, so helper methods will be added to simplify this.

Actions now result in 3 artifacts per report: A markdown file, a PDF, and an HTML file. The collapsible details blocks supported by markdown don't work well with the other output formats, so they have been removed and replaced with a table of contents for easier navigation.

Sample proposal reports can be found in the artifacts of this workflow run: https://github.com/Uniswap/governance-seatbelt/actions/runs/2515723372

The HTML reports have two issues:

@mds1 mds1 marked this pull request as ready for review June 17, 2022 12:59
@@ -0,0 +1,213 @@
import fs, { promises as fsp } from 'fs'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was renamed from markdown.ts

@mds1
Copy link
Contributor Author

mds1 commented Jun 27, 2022

Successful test CI run in https://github.com/Uniswap/governance-seatbelt/actions/runs/2569653666, so merging this

@mds1 mds1 merged commit ebc9616 into main Jun 27, 2022
@mds1 mds1 deleted the report-improvements branch June 27, 2022 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reports: clean up the info, warnings, errors part of proposal checks
2 participants