Skip to content
This repository has been archived by the owner on Jun 30, 2022. It is now read-only.

Refactor the code #51

Open
joao-paulo-parity opened this issue Sep 2, 2021 · 5 comments
Open

Refactor the code #51

joao-paulo-parity opened this issue Sep 2, 2021 · 5 comments

Comments

@joao-paulo-parity
Copy link
Contributor

I've pointed out many times (e.g. #10 (comment)) that this repository's code is not in a good state and needs some refactor.

My current idea is to rework try-runtime-bot's module structure so that there would be

  • A "core" of shared functionality functionality e.g. the command executor, prepareBranch, etc).
  • Separate folders "bench-bot" and "try-runtime-bot" which can be deployed and built separately.

To make it extra clear, it would not mean the bots would be merged into one, just that we'd have a monorepo for sharing tooling and functionality between them. e.g. prepareBranch is implemented in both bots at the moment - whenever I have to fix or make some improvement here, often there's overlap in functionality which would be useful for both bots.

This move also implies bench-bot would be rewritten in TypeScript, which is not costly at the moment because the bot is pretty simple and try-runtime-bot's code does not have a huge amount of technical debt.

From the 2.0 specification, the following are already implemented in try-runtime-bot:

  • Access control (ALLOWED_ORGANIZATIONS)
  • Queue
  • Cancel
@joao-paulo-parity
Copy link
Contributor Author

Another option, instead of refactoring, is to rewrite the bot in Rust if our recurring problem of "Node.js async API overhead" (#63) doesn't get solved anytime soon

@Vovke Vovke added duplicate This issue or pull request already exists and removed duplicate This issue or pull request already exists labels Apr 12, 2022
@mordamax
Copy link

mordamax commented May 4, 2022

@joao-paulo-parity taking #105 to account - we probably can reduce this to only ACL here, right?

@joao-paulo-parity
Copy link
Contributor Author

@mordamax This is unrelated to https://github.com/paritytech/opstooling/issues/105. This issue is for refactoring the current bench-bot while https://github.com/paritytech/opstooling/issues/105 would be a different implementation.

@mordamax
Copy link

mordamax commented May 5, 2022

@joao-paulo-parity I understand, but won't integrating bench-bot with paritytech/opstooling#105 remove the need in Queue and Cancel parts?

@joao-paulo-parity
Copy link
Contributor Author

joao-paulo-parity commented May 5, 2022

@mordamax my point is that issues in this repository are relevant for how bench-bot currently works. The new implementation of https://github.com/paritytech/opstooling/issues/105 will be in a different repository, so most tickets here are out of reach at least for the MVP.

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

No branches or pull requests

3 participants