taoeffect
commented
4 years ago Thank you Steven for sharing your preferences on how this project should operate!
We use a problem/solution format because it allows us to discuss and focus on real problems, one at a time. It also lets us analyze whether something is in fact a problem (or not), and its potential solutions. If you don't have a solution, that isn't a reason to not open such an issue. Others might have a proposed solution that they can comment with, or it might turn out to be the case that the problem isn't a problem at all.
You've raised 14 different concerns here. If you feel strongly about any one of them, open an issue for it, and allow people who are interested to focus on it in that issue. At present, a discussion in this issue between all team members will be close to impossible, because there are so many totally different topics to discuss, and it would fill up email inboxes. For these reasons and more, we use the Problem/Solution format, and we ask that everyone adhere to it.
For example, if you believe that we shouldn't do code reviews, then open an issue for that, and tag it with Kind:Process. Then we can discuss what the problem is, and what the solution is.
Some of the suggestions you bring up very well may have merit to them. Perhaps Prettier is better than StandardJS. Who knows? This is something we as a team can discuss and collectively decide together. If you feel strongly about Prettier, open a Problem/Solution issue for it and tag it with Note:Tooling.
Some of these topics we've discussed before, for example, SBP, project configuration, project goals, the database, etc. We even came to conclusions on these topics, for example, with the database I thought it was clear that our solution is the simpler solution, and that we do not have any need or use for something like MySQL, which would not only make the project far more complicated (for no reason), but it would also make it much more difficult to protect user data. I will spare others a rehashing of the other subjects, but if anyone would like me to elaborate on them please feel free to post a comment below to the GH discussion or on Slack/Gitter.
Another reason we use Problem/Solution format is so that we can assign issues to people to work on. It's not really possible to assign this issue to anyone, because it amounts to a list of personal preferences that you'd like to see implemented. And a lot of your preferences I agree with! For example, I agree that business logic should be de-coupled from the UI framework. In fact we are doing that already in the gi.actions SBP namespace. As you mention, it will be usable from a CLI, and that is a rule-of-thumb we use. We even have an existing issue for it: #749
So, thank you for your thoughts and feedback! It is always appreciated because anytime anyone sees a way to improve the project, we're interested in hearing from them. However, we do ask that everyone respect our process, which isn't even ours per-se, because we adopted it from the C4.1 system invented by Pieter Hintjens from his ZeroMQ project. A lot of thought and reasoning has gone into every single decision we've made over the past 4 or 5 years by all of the team members, and newcomers to the project might not be familiar with that reasoning.
I'm going to close this issue because it isn't in the Problem/Solution format that we require, but that doesn't mean that discussion isn't welcomed. @sandrina-p recently pointed out that Github has a discussions feature that we can use to avoid filling up people's Inboxes with notifications. So maybe let's give that a try?
Feel free to continue the discussion there!
thesoftwarephilosopher
sandrina-p
Hi team. I have some constructive feedback that I've come up with based on my experiences working on this project.
Simplify project features
A good MVP has minimal features, focuses on the essentials, and puts off extras until later. Essentials for this project would be things like proper user accounts, real email-verification, implementing data persistence, and the ability to actually send payments from the app, which it currently has none of. Extras would be things like real-time updates in clients and the chat feature. These should have waited until after public release.
Remove SBP
SBP is just a a much more complicated version of global function calls, with a little bit of RCP mixed in. Using regular ES modules would be far simpler, and make the code much cleaner and easier to work with. It would also have better automatic integration with IDEs. The RCP and logging concepts can be done in other ways that are orthogonal to ES modules and don't introduce this serious complexity, and aren't invasive in the implementation of the code.
And it's not actually an advantage to not have to import functions in order to call them. That leads to implicit dependencies, and it breaks IDE support, where you can Ctrl-Click to go to a function definition. This is extremely useful. And modern IDEs have support to automatically update import paths when you move/rename a function or file.
Decouple logic from Vue
It's generally considered very bad practice to couple business logic so tightly with an external framework. This makes it very difficult to refactor or reuse code, to test business logic, or to structure code properly. I strongly recommend pulling all the business logic out into a group of pure JS functions with its own separate structure, and then call this business-logic pure-JS app from Vue, without ever doing any real logic in Vue that's not puely related to browsers. The rule of thumb is, make sure your business logic can be called from a CLI utility, curl, or a web browser, without ever repeating any code.
Refocus tests
Relating to the last one, Cypress tests should only be smoke tests for a few hot spots in the UI, just to make sure everything is hooked up right. The vast majority of the tests should be written in pure JS for the pure JS business logic. Knowing whether a payment was considered to be partial or not should not be something that requires running the actual UI to figure out.
Simplify configuration
Simple things like supporting project-relative import paths make it hard to integrate with IDEs and add up to a non-trivial amount of configuration. The gruntfile is about 500 lines of code. The general rule of thumb is, the more you customize something, the more you break everyone else's efforts to integrate with it, and the more you glue things together, the less of other people's glue you can use. And generally, their glue and integrations are generally better quality and better tested than any in-house solutions.
I've seen many small and big companies follow this pattern over the past 10-15 years and gradually move more and more to third party solutions, decreasing their in-house integrations. For an app this size, there could be almost no configuration necessary. Besides, your configurations are causing more issues, such as the one where build times increase gradually while developing.
I admit I tend on the side of zero-configuration (see this React example), which is extreme, and I'm not necessarily suggesting that in this app's case, since it might not work out that great with Vue (but it might) and TypeScript (though JSDocs helps a lot). But I'm positive at least a good amount of configuration could be simplified or even removed here, relative to my other suggestions.
Browser devtools and debuggers are extremely useful, and configuration that works against these or even completely breaks integration with these should be avoided.
Focus on the app
Group Income started out as an idea for an app, and at some point the idea of a protocol creeped in, but it makes no sense in this context. Most people do not want a protocol for everything, they want an app. Forgetting about the protocol will allow simplifying a lot of the project goals and also a lot of the project implementation.
Simplify project goals
This project has long-term goals of becoming an entire software platform and even OS for subsets of society, hence SBP and the chat feature, and that's just not a good reason to add these features and implementation complexities into the app this early, while it's not even released.
Use simpler decentralization
Just use a regular database and allow people to host their own instance of the server. One reason I was told this was avoided was because it would also need to be secured, but the amount of effort that went into creating the current decentralization setup far surpasses what would have been needed to just make sure an open source NoSQL database is securely used by the server.
Use TypeScript
It has basically won. It's only a matter of time until you switch over to it, might as well do it now while the Flow setup is already completely broken. Having proper types is really essential for making an app this size, so I would focus on this first, getting the types working and typing everything. At least in the pure-JS side of things (see the Decoupling from Vue section). The longer you put this off, the harder it will be to add types later.
Avoid over-optimization
Some of the algorithms, which are already hard to follow, were made much harder by optimizing to avoid looping twice instead of once, etc. This is very detrimental to productivity. Algorithms are almost never the real bottleneck. Code at this level should be written as closely as possible to how we actually think of the problem and should follow our mental model closely.
Code reviews
I would honestly rethink the PR workflow here, it probably isn't necessary to have reviews at all. Let trusted core devs work on master together and communicate/coordinate to avoid complex conflicts, and just do PRs for third party submitters.
Don't avoid classes
Classes are not automatically bad. Inheritance is generally not the best solution, but often classes are a good and useful way to orgnanize the relationship between methods and the data they operate on. They also play well with browser tools and optimizations, making it much easier to inspect their data in e.g. a browser's devtools, than it is to inspect lexically captured values in a closure. FP and OOP are not better or worse, they'e complementary.
Use Prettier instead of StandardJS
I get the idea of wanting to avoid thinking about formatting and styles. And that's actually why I gave up on trying to format my own code and just let VS Code format it for me on save, which internally uses Prettier. This is subjective but StandardJS makes weird style choices and Prettier's defaults in VS Code seem much more reasonable and less strict.
Issue format
It doesn't always make sense to put things in problem/solution format. Maybe the person doesn't have a solution, or doesn't have time to think of a solution, or the solution is obvious and implied. Their feedback is still valuable and you discourage it by requiring this format when they can't adhere to it.