paritytech / polkadot-sdk-docs

_THE_ Polkadot SDK Tutorial
19 stars 0 forks source link

Identify API + team to review it #2

Closed kianenigma closed 1 year ago

kianenigma commented 1 year ago

requirement for developers to identify if their PR is touching the application programming interface aka. API of our software, or our documentations (to the best of their knowledge) and if so:

  • pay extra attention to the code in that PR being well documented.
  • flag the PR (through either a request review, label, or similar) to be audited/reviewed by the those who maintain documentation.

For the time being, I think the easiest would be to use code-owners rules and assign a team to auto-review PRs if they touch a particular set of folders.

Might be a bit noisy, as this will be triggered for all non-draft PRs.

hummusonrails commented 1 year ago

@kianenigma Do you want me to try and set up an initial conversation with the Swimm team and see if their open source solution might be able to assist us with this? I think a lot of what their product does is exactly in this area.

kianenigma commented 1 year ago

Do you know if/how they can complement the above scenario?

Teams and actions are probably enough for now. I think we can set that up and try the whole process, and then see if we can improve it via Swimm.

I am kinda curious if the whole "DevRel audit" system works, and would like to set that up sooner rather than later.

kianenigma commented 1 year ago

Given that I really want to kickstart this ASAP, here's my quick suggestion of how we can start on this:

The API

More candidates, but maybe skip for now:

An alternative way to think about this, inspired by something that @athei once shared with me, would actually to create a single new create called frame-api. This crate will have ZERO code, and ONLY re-exports parts of substrate that you need to build a runtime with.

Whatever ends up being in this, ends up being well documented. It will also help us towards semver and so on.

I like the idea, but I am worried that it is me being too ambitious here and fixing too many things at once. Also, it is probably way more easier said than done.

ggwpez commented 1 year ago
  • everything in frame, except

Might as well put them in the CODEOWNER file so the review request is automatic? Or ask CICD team for custom review GHA.

frame-api

Would be nice to have, since i think there are Rust tools that can detect breaking API changes - at least to function signature and returns. So even if this stays internal, could be useful to have it aggregated.

kianenigma commented 1 year ago

Might as well put them in the CODEOWNER file so the review request is automatic? Or ask CICD team for custom review GHA.

Initially CODEOWNERS should do, but later, if we see the system working we might need a custom review rule to eg. prevent merge.