semantic-release / cli

:cl::round_pushpin: Setup automated semver compliant package publishing
MIT License
367 stars 45 forks source link

Refactoring #174

Open kbrandwijk opened 6 years ago

kbrandwijk commented 6 years ago

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.

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.

gr2m commented 6 years ago

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 🙌

kbrandwijk commented 6 years ago

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.

gr2m commented 6 years ago

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?

kbrandwijk commented 6 years ago

Proof of Concept.