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

feat: Publish static reports via github pages #68

Merged
merged 33 commits into from
Jun 14, 2022
Merged

Conversation

SgtPooki
Copy link
Member

No description provided.

@SgtPooki
Copy link
Member Author

SgtPooki commented Jun 1, 2022

workflow ran successfully for both "schedule", emulated via workflow dispatch, and for PR

see the results at https://ipfs-shipyard.github.io/pinning-service-compliance/

Copy link

@BigLep BigLep left a comment

Choose a reason for hiding this comment

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

You already called out "I’ve gotta fix the rendering for the summary section, I think I just need some extra spacing"

I think the other key thing that would be useful to have in the reports are:

Date that the report was generated. I assume that is gleanable from page history, but it's really useful information to have front and center for someone.
commit of compliance checkeker code that was used (for reproducibilitiy). That said, not required as derivable from the "run date"

.github/workflows/build-and-publish-reports.yml Outdated Show resolved Hide resolved
.github/workflows/build-and-publish-reports.yml Outdated Show resolved Hide resolved
.github/workflows/build-and-publish-reports.yml Outdated Show resolved Hide resolved
.github/workflows/build-and-publish-reports.yml Outdated Show resolved Hide resolved
@SgtPooki
Copy link
Member Author

SgtPooki commented Jun 2, 2022

Date that the report was generated. I assume that is gleanable from page history, but it's really useful information to have front and center for someone.

Good idea. I will update to include the date.

lidel
lidel previously requested changes Jun 6, 2022
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thank you for figuring this out!

Agree with feedback from @BigLep

  • nft and web3 need to be renamed to include storage suffix
  • UTC date and version of pinning-service-compliance used for generating the report.

Filled issues for small UX nits: #77 and #76 – these are small enough that you may want to include them here too.

@SgtPooki
Copy link
Member Author

SgtPooki commented Jun 6, 2022

@lidel @BigLep check out https://ipfs-shipyard.github.io/pinning-service-compliance/api.estuary.tech.html and current reports if you would. date and revision with link to github commit responsible for report are there now.

@BigLep
Copy link

BigLep commented Jun 6, 2022

@lidel @BigLep check out ipfs-shipyard.github.io/pinning-service-compliance/api.estuary.tech.html and current reports if you would. date and revision with link to github commit responsible for report are there now.

@SgtPooki : cool - great to see! A couple of minor ideas but aren't blocking:

  1. "Date" to "Execution Date" to make it clear what this date is.
  2. "Revision: Revision:" to "Revision:"
  3. Can the summary section be links to the corresponding sections below?
  4. Maybe add a link for how someone can easily see historical run results (I assume a link to the commit history of the page?)

@SgtPooki
Copy link
Member Author

SgtPooki commented Jun 6, 2022

@lidel @BigLep check out ipfs-shipyard.github.io/pinning-service-compliance/api.estuary.tech.html and current reports if you would. date and revision with link to github commit responsible for report are there now.

@SgtPooki : cool - great to see! A couple of minor ideas but aren't blocking:

  1. "Date" to "Execution Date" to make it clear what this date is.

Absolutely. quick fix.

  1. "Revision: Revision:" to "Revision:"

oops, good catch

  1. Can the summary section be links to the corresponding sections below?

Yea, that should be easy.. let me give that a go.

  1. Maybe add a link for how someone can easily see historical run results (I assume a link to the commit history of the page?)

historical run results.. that could be useful.. I'm using git-rev from npm currently, and I could easily add a (last report) link.. but linking to previous results would just be linking to the gh-pages branch commit log.

I'm not sure that the s0/git-publish-subdir-action@399aab378450f99b7de6767f62b0d1dbfcb58b53 action keeps history intact though.. and there's no other mechanism. So if gh-pages branch doesn't maintain history, then this feature would be a large effort

@SgtPooki
Copy link
Member Author

SgtPooki commented Jun 6, 2022

looks like the publish subdir action will allow you to keep history via SQUASH_HISTORY: https://github.com/s0/git-publish-subdir-action#configuration

@BigLep
Copy link

BigLep commented Jun 6, 2022

Per 2022-06-06 verbal: one other idea is to move the "Joi validation failures" (example) to just be a set of bullets inline under "Response object matches api spec schema (failure)". The reasons for doing this:

  1. It prevents us from rendering "Joi validation failures" when there aren't any
  2. It puts them in context for where they're relevant. Otherwise folks are left wondering, "why are Joi validation failures, why are they relevant?"

@SgtPooki
Copy link
Member Author

SgtPooki commented Jun 7, 2022

ok @BigLep @lidel , i made more than a few updates. check out https://ipfs-shipyard.github.io/pinning-service-compliance/api.estuary.tech.html

@BigLep
Copy link

BigLep commented Jun 7, 2022

@SgtPooki : I don't have any comment on the code itself.

Two things:

  1. I see "Previous Revision: (Error getting previous revision)" at the top. Not sure if that's expected. I think jus having a link to the report history is fine without a direct link to the previous revision.
  2. Using https://ipfs-shipyard.github.io/pinning-service-compliance/api.estuary.tech.html#joi-validation-failures-2 as an example, what about renaming "Joi validation failures" to "Response object validation failures with api spec schema" to make it more clear and ideally don't render it if we don't have anything. This seems like something quick/cheap until Report output: potential updates to "Joi validation failures" output #78 is done.

@SgtPooki
Copy link
Member Author

SgtPooki commented Jun 7, 2022

  1. I see "Previous Revision: (Error getting previous revision)" at the top. Not sure if that's expected. I think jus having a link to the report history is fine without a direct link to the previous revision.

That is expected, but I can remove it.. because it won't work from NPX nor github actions apparently because checkout only checks out the latest revision. I agree, now that Revision history is there, it's not as useful even if it were to show up

2. Using https://ipfs-shipyard.github.io/pinning-service-compliance/api.estuary.tech.html#joi-validation-failures-2 as an example, what about renaming "Joi validation failures" to "Response object validation failures with api spec schema" to make it more clear and ideally don't render it if we don't have anything. This seems like something quick/cheap until Report output: potential updates to "Joi validation failures" output #78 is done.

What about "Response object doesn't match compliance spec schema:"

@SgtPooki
Copy link
Member Author

SgtPooki commented Jun 7, 2022

or "Response object doesn't match expected schema:"

@BigLep
Copy link

BigLep commented Jun 7, 2022

"Response object doesn't match expected schema:" sounds good. It's more clear than "Joi validation failures" :)

@SgtPooki SgtPooki marked this pull request as ready for review June 8, 2022 19:39
@SgtPooki SgtPooki requested review from lidel and BigLep June 8, 2022 19:39
Copy link

@BigLep BigLep left a comment

Choose a reason for hiding this comment

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

Looks good to me from a presentation side. I didn't review the code.

@SgtPooki SgtPooki dismissed lidel’s stale review June 13, 2022 17:21

changes were made

@SgtPooki SgtPooki self-assigned this Jun 13, 2022
@lidel lidel mentioned this pull request Jun 13, 2022
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

lgtm, thank you @SgtPooki

Small nit / question below, but ok to merge as-is.

src/output/getReportEntry.ts Outdated Show resolved Hide resolved
SgtPooki and others added 2 commits June 13, 2022 13:41
This makes things easier to reason about:
In this order, each step has more strictness.
@lidel
Copy link
Member

lidel commented Jun 14, 2022

I reordered things in Request/Response section (a5f8eff) but no functional changes.

@SgtPooki merge if it looks ok to you 🚀

@SgtPooki SgtPooki linked an issue Jun 14, 2022 that may be closed by this pull request
@SgtPooki SgtPooki changed the title Update build-and-publish-reports.yml feat: Publish static reports via github pages Jun 14, 2022
@SgtPooki SgtPooki merged commit 5a6a7f5 into main Jun 14, 2022
@SgtPooki SgtPooki deleted the SgtPooki-patch-1 branch June 14, 2022 18:38
github-actions bot pushed a commit that referenced this pull request Jun 14, 2022
## 1.0.0 (2022-06-14)

### Features

* add npm bin to devcontainer PATH ([365c142](365c142))
* auto-update github actions with dependabot ([#43](#43)) ([87f7926](87f7926))
* compliance check infrastructure ([7aa5663](7aa5663))
* Export esm module ([#41](#41)) ([acaeac6](acaeac6))
* implement all compliance checks ([#17](#17)) ([1223831](1223831)), closes [#5](#5) [#4](#4) [#6](#6) [#7](#7) [#8](#8) [#28](#28) [#25](#25)
* lplaceholder checks run via listr ([bbb1f81](bbb1f81))
* Publish static reports via github pages ([#68](#68)) ([5a6a7f5](5a6a7f5)), closes [#78](#78) [#76](#76) [#77](#77) [#77](#77)

### Bug Fixes

* CI release succeeds ([#90](#90)) ([810d4a2](810d4a2))
* Compliance check sum is consistent for all services ([#54](#54)) ([ace2b57](ace2b57)), closes [#55](#55)
* devcontainer sees pinning-service-client ([c7a6dbb](c7a6dbb))

### Trivial Changes

* **deps-dev:** bump @types/node from 17.0.25 to 17.0.34 ([#49](#49)) ([a2a5d13](a2a5d13))
* **deps-dev:** bump @types/node from 17.0.35 to 17.0.41 ([#79](#79)) ([db8a6f7](db8a6f7))
* **deps-dev:** bump aegir from 37.0.15 to 37.2.0 ([#86](#86)) ([df4035f](df4035f))
* **deps-dev:** bump ipfs-core-types from 0.10.3 to 0.11.0 ([#64](#64)) ([e623ad3](e623ad3))
* **deps-dev:** bump ts-node from 10.7.0 to 10.8.1 ([#75](#75)) ([ff48f3b](ff48f3b))
* **deps:** bump actions/checkout from 2 to 3 ([#46](#46)) ([8fbbcd8](8fbbcd8))
* **deps:** bump actions/setup-node from 2 to 3 ([#48](#48)) ([ee57c59](ee57c59))
* **deps:** bump github/codeql-action from 1 to 2 ([#47](#47)) ([1dd488d](1dd488d))
* **deps:** bump go-ipfs from 0.12.2 to 0.13.0 ([#80](#80)) ([baaaa8c](baaaa8c))
* **deps:** bump ipfsd-ctl from 10.0.6 to 11.0.1 ([#58](#58)) ([45771db](45771db))
* **deps:** bump lewagon/wait-on-check-action from 0.2 to 1.1.1 ([#44](#44)) ([9c8f5c0](9c8f5c0))
* **deps:** bump node-fetch from 3.2.4 to 3.2.6 ([#81](#81)) ([5bcb760](5bcb760))
* **deps:** bump pascalgn/automerge-action from 0.13.1 to 0.15.3 ([#45](#45)) ([65c19dc](65c19dc))
* getting started instructions ([b7ff148](b7ff148))
* ignore .envrc ([54e5aae](54e5aae))
* static report landing page provides context ([#87](#87)) ([12fc841](12fc841))
* use gitignore.io ([e81ebd9](e81ebd9))
@github-actions
Copy link

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create static output of pinning service compliance checks
3 participants