search

paritytech / bench-bot

ISC License
9 stars 19 forks source link

Refactor the code #51

Open joao-paulo-parity opened 3 years ago

joao-paulo-parity commented 3 years ago

I've pointed out many times (e.g. https://github.com/paritytech/bench-bot/pull/10#issuecomment-842011715) 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

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:

joao-paulo-parity commented 2 years ago

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

mordamax commented 2 years ago

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

joao-paulo-parity commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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.