Feature/hck 3426 re couchbase 7 instance re

[serhii-filonenko]

HCK-3426 RE Couchbase 7+ instance with scopes and collections

Implemented:

• Retriving document kinds for documents located in _.default._default collection
• RE collections and defined document kinds as collections including indexes
• RE from n1ql files

[bigorn0]

What is this file about? buildConstants is it for esbuild?

[bigorn0]

I'll post a general remark here to highlight:

@Serhii Filonenko
There is something i’d like to raise
Organizing files is good that’s a fact but folders with a name like helpers and files inside named like Helper.js doesn’t really bring anything. It is just generic meaningless naming.
There are a few questions you could ask yourself that can help structure the code properly with meaningfull and better names:

• do the whole logic belongs only to reverse engineering, forward engineering or is it something of higher abstraction level? Here we are really mixing connection, ddl/storage structure (e.g objects and hierarchy), discovery with queries, and utility function like error handling.
• are the functions inside the files really focused on a single purpose (for instance if I look at clusterHelper.js all functions seem either related to buckets and some others are to discover top level structure of the Couchbase storage. So in terms of organization that already looks fishy (i would have a bucket.js and extract unrelated functions in other smaller modules for example a couchbaseErrorHandler.js or something in that vein, didn't spend much time reflecting on it)
• what are the real semantic concepts (like here many things are mixed up), i see functions to extract and discover database/storage structure and hierarchy e.g. Data Structure Definition (e.g DDL discovery). From my pov DDL/data structure should be top level and is not owned by RE nor FE, they both use DDL one way or another but they don’t own the logic.
• Ownership is important.
  Don’t hesitate to have targetted folders with small focused modules with a super well focus and defined scope (it’s fine to have a module of a few lines because it helps tree shaking during bundling eventually and improve module isolation (purity?) and avoid unwanted side effects (+ easier unit testing)

cc @chulanovskyi-bs

[chulanovskyi-bs]

@bigorn0 Hi Ugo!

Couldn't agree with you more, that we need to strive for a well-organized codebase as much as it makes sense. However, I don't think that "helpers" folder is completely meaningless, because devs can quickly detect these folders as a place for commonly used functions. Definitely, there is a room for decoupling the mentioned cluster.js file down to more granular modules of responsibilities. But it also would require another deep round of refactoring of the logic itself. From what we've been discussing with the team, this version of a reorganized code in comparison with what we had previously in the studio repo - is way more readable and understandable even with such unideal decoupling. So in order to provide iterative deliverables - it looks "fine" for now.
I think the points that you've shared in a PM over slack are extremely valuable and worth to be captured in a generalized form on our Architectural Guidelines page. It also may include examples of how to "do / do not" do in particular specific cases. That could be a starting place where everyone in the team could align the expectations about how to organize their code in any plugin repo. What do you think?

[bigorn0]

Looking at the content of helpers folder it is quite obvious the core logic is about retrieving/discovering database objects and hierarchy that we try to model so definitly, helpers has nothing to do with it. I could name it <blah>, it would give the same information. This initial implementation is a good opportunity for a change even if it is burried from app or older logic. I'm fine with taking this as a first step as long as code restructuration and clarification of the api come right after as a follow up step.

About the fact that dev can quickly identify commonly used functions then again, i don't get it because most of the functions inside that helpers folder are specific to Couchbase. Most of the functions inside are following an interface and implement what's expected by the application (REWorker scenario) for a given database structure discovery workflow then it is best to organize and surface that fact. I think there are enough abstractions and technics in Javascript to achieve that goal even without Typescript.

https://app.gitbook.com/o/HBtg1gLTy0nw4NaX0MaV/s/k3cNzEeXGnIxGNnOQdDa/architecture/architectural-guidelines Already promotes the small, focused, well scoped module vs the GOD object approach that helpers, services, tools and other similarly named modules eventually become.

[chulanovskyi-bs]

@bigorn0 Thank you for your feedback Ugo, the argumentation does make sense for me as well. We surely plan to continue refactoring that part of the implementation in next iterations. Again, the main intention is to deliver the working functionality that can be tested and verified, not delaying the release with an infinite rounds of reengineering it 👍🏻

[bigorn0]

Good, then I'm eager to see iteration 2 :)

[chulanovskyi-bs]

@bigorn0 Could you please check what's happening with the actions config? Seems like something misconfigured:

Run PR tests (Plugins) (Plugins)