search

ocurrent / opam-repo-ci

An OCurrent pipeline for testing submissions to opam-repository
19 stars 21 forks source link

Extract analysis from OCurrent #323

Closed benmandrew closed 1 week ago

benmandrew commented 1 month ago

Many components of opam-repo-ci are tightly coupled to OCurrent, making it difficult to use outside of a pipeline. Ideally OCurrent should wrap local tools, such as the linting check.

This PR pulls out the OCurrent-specific bits from the analysis step, functorising over the implementations of logging and executing commands.

I will open another PR doing the same for the linting check, but the linter takes in data from analysis so this PR must be done first before we can get a fully non-OCurrent linting tool.

shonfeder commented 1 month ago

This is just an initial impression, so please weigh it accordingly. But but my first thought is that this does not seem to be the preferred route to having local-first tools. I.e., we are not aiming for a situation where users can build and run opam-repo-ci locally; rather, we want to replace much of the opam-repo-ci business logic with a CLI tool. As I understand things, we have both pending plans and WIP in this direction so I'd like to be a bit cautious on how we move forward here. We should make sure it fits with our roadmap before we go introducing the complexity of functorizations over everything.

Before you spend more time on this, I suggest we talk it over in our team meeting.

shonfeder commented 1 month ago

I framed my initial thinking and reply to this PR poorly. I ought not to make suggestions (in that manner) about how you are spending your time and I believe that was out of order. I'm sorry about that!

What I wish I had meant to say, and what I now do mean and would like to say, is this:

I agree that decoupling is helpful, and that is a prerequisite for supporting local-first functionality. I'd like to understand your design idea for this better, and I think it would be helpful me if we could discuss it.

shonfeder commented 1 week ago

I'm going to close this for now, as moving forward with this course of refactoring isn't aligned with our current strategy to break the logic out into independent tools that can be run locally. But it will be useful to have this branch for reference. Thanks, Ben!