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

Automated Review #3

Open
dlamkins opened this issue Jun 25, 2020 · 4 comments
Open

Automated Review #3

dlamkins opened this issue Jun 25, 2020 · 4 comments
Labels
documentation Improvements or additions to documentation.

Comments

@dlamkins
Copy link
Member

dlamkins commented Jun 25, 2020

Automation through CI should be the primary method of PR review to ensure a minimum level of quality in submitted manifests, reduce the chance for human mistakes, and reduce the workload of reviewers.

@dlamkins
Copy link
Member Author

I propose that the items following should be automated when a user PRs a new module package manifest. Any test failures would consequently block merging until resolved.

  • Manifest validity — ensure manifest is parsable and meets schema requirements.
  • Location check — ensure BHM is available via the location provided in the manifest.
  • Module checksum — ensure the SHA256 hash of the BHM matches what has been provided in the manifest.
  • VirusTotal scan — using the VirusTotal public API, ensure that the linked .bhm is not flagged after scanned.

Other potential items:

  • BHM check — ensure that the format of the linked .bhm file matches what is expected of a module (contains a manifest, the DLL name matches the package name listed in the manifest, etc.
  • Dependency redundancy check — ensure that the module doesn't include redundant dependencies in the module (avoid packaging, for instance, Blish HUD itself or Gw2Sharp).

Some of these items are inspired by the automation utilized in the winget-pkgs repository for their submission validation.

@greaka
Copy link
Member

greaka commented Jul 1, 2020

We could additionally check if the latest supported bhud crashes when the bhm with all its dependencies gets loaded. It would be a lot of work to implement this though.

@dlamkins
Copy link
Member Author

dlamkins commented Jul 1, 2020

I absolutely see value to that, so I don't want to discount it, but you're right — it would be some extra work. I suspect that is something we could implement down the road, though.

@dlamkins dlamkins added the documentation Improvements or additions to documentation. label Feb 4, 2021
@dlamkins
Copy link
Member Author

dlamkins commented Nov 1, 2021

The first implementation of this is complete.

image

It is not yet triggered automatically, but does cover the following tests:

  • Checks that module URL is correct.
  • Checks that module namespace and package namespace match.
  • Checks that module version and package version match.
  • Checks that module checksum matches the one specified in the package manifest.
  • Ensure DLL isn't missing within the bhm.

It also hosts decompiled source and ref contents in a repo to make reviewing code changes easier.

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

No branches or pull requests

2 participants