cli icon indicating copy to clipboard operation
cli copied to clipboard

Refactoring

Open kbrandwijk opened this issue 8 years ago • 4 comments

This issue is to gather requirements for the refactoring of the CLI codebase

Functional

Adding more and more features to the CLI means that the flow becomes less and less clear, and harder to maintain. This became apparent with the latest and planned additions to the npm flow. I want to just put a few ideas out here, to get feedback, and to eventually come up with the right approach.

At the root of the system, we have a tree of interrelated questions, with the purpose of gathering enough information to set up CI. As these questions are interrelated, they cannot be simply statically defined, but need to be determined at runtime based on the course of events.

Technical

The current codebase is highly procedural, basically it's a big if-then-else tree (using both Inquirer's when pattern, and conditional logic in handlers). Inquirer supports the use of Reactive programming through RxJS. This decouples the asking of questions and the processing of answers in a nice way. The Reactive principles are very powerful, and I believe they would be a good fit for this use case.

Based on these principles, each of the components (npm, git, ci) could use the same structure, and it will be a lot easier to extract some of the 'plumbing' logic into reusable components.

  • [ ] Create base class for components
  • [ ] Create class for each component
  • [ ] Use Inquirer's Reactive approach
  • [ ] Apply the same Reactive approach to the handling part

Other areas

There are other areas of the CLI that could do with some rework (like the command part), but I'd like to focus on the core functionality first, because every addition (Gitlab, GHE, Circle, npme) makes it harder to maintain the current solution.

kbrandwijk avatar Jan 08 '18 18:01 kbrandwijk

I never worked with RxJS myself, could you maybe create a minimal demo repository just to demonstrate how you would like to structure the CLI using it? I’d be very curious to see it

In general if you are excited about it and commit to be there if people run into any problems introduced by the refactoring, I have no objections. I’d only ask for full test coverage. In terms of priorities: ease of contribution and maintenance from my side.

Thanks for championing this Kim šŸ™Œ

gr2m avatar Jan 08 '18 21:01 gr2m

After doing a lot more research into this, and a small PoC, I have come to the conclusion that the Reactive pattern is not suitable for our use cases. Especially the conditional asking of additional questions does not play well at all with the Reactive components. I will rethink the refactoring based on a better use of the inquirer, and better splitting up of functional blocks.

kbrandwijk avatar Jan 09 '18 15:01 kbrandwijk

thanks for doing the research @kbrandwijk. The people who change their opinion are the people who are right most often šŸ‘

What does "PoC" stand for?

gr2m avatar Jan 13 '18 20:01 gr2m

Proof of Concept.

kbrandwijk avatar Jan 13 '18 23:01 kbrandwijk