-
Notifications
You must be signed in to change notification settings - Fork 9.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[doc] contributing guide #3
Comments
Hello! Thanks a lot for creating an issue about that! And you are 100% right. It is currently more than light. Actually is the documentation one of the main reasons I did not launch n8n anywhere officially yet. Hope to imporove on that a lot till the end of this month when I want to launch it on some networks. Will look tonight into your issue and get then back to you! |
Hello! I checked now. There was a problem with the build because inquirer. Somehow was not added as dependency at all (right now no idea when it got lost and why the docker builds still worked fine). Anyway that did not even cause the problem, it was actually that they apparently renamed a type and therefor it did not build anymore. After I fixed that issues and followed the instructions everything worked fine for me. I am however on Ubuntu and you seem to work on Windows, so maybe something goes wrong there. Btw. lerna does not has to be installed for the instructions to work. I extra used Will now check in Windows and then get back to you. |
"Sadly" no luck. It also worked without a problem on Windows for me. I however now "fixed" the However, the error message you get and the fact that for you the |
I'm at home now, on Linux Mint, did:
and the build went fine. However, I can reproduce my previous issue if I do:
Error:
I did not have the issue with the commas, so that might be related to Windows. But the later error remains. Here are my globally installed npm packages on Linux Mint:
On the subject of welcoming potential contributors, a link to a short video explaining what lerna is and what it does would be nice. Also a short description of what role have the different packages would help ;) |
Ah very great that it works now! If you check out the old version on Linux you actually do not reproduce the previous issue. If you compare the error messages they are totally different. The one you get on Linux is the one I talked about before with that inquirer renamed a type "inquirer.Questions" -> "inquirer.QuestionCollection". Thanks a lot for the feedback! Will add it to my list and improve it the next days. And btw. welcome to the project. Great to have you on board! |
Improved now the documentation: If you think anything else should be added please simply add it. A pair of fresh eyes can judge that for sure much better. Thanks! |
Just another though regarding contributing (and attracting contributors). I'm not convinced by the monorepo thing (and need for lerna to handle it). I understand a maintainer of the project would have a better life with the monorepo form and lerna. But on the other hand, wanna-be-contributors are firstly users of the app who are missing a features or are annoyed by a bug. Such wanna-be-contributors won't have much time to learn how to use lerna and to discover how things are organised and linked together in a monorepo, because it won't bring much value to them. That might be a deterrent. I guess users try to become contributors with the following cases :
|
Guess there it is how it is with almost all things. Everything has its advantages and disadvantages. So I kind of fear that changing that would replace one problem with an even bigger one. For sure would make it easier for one time contributors but would drive the more involved people crazy. At least that is what I think would happen. If there is however a good solution for things like mentioned above, I am more than open for any better solution. Is not that I think mono repro is always the way to go but not without reason are more and more projects switching to it and not without reason have also one of the biggest companies in tech mono repositories. |
Keeping the monorepo, the contributing documentation could welcome new users with shortcuts for frequent contribution cases like :
giving for each of those cases quick instructions on how to get setup without explaining to much about the other parts of the monorepo project. |
Yes, that sounds like a very good idea! Also think that could help new contributors a lot and so hopefully attract more. |
INT-483: Add missing endpoints to n8n-onfleet node
User module gRPC setup + others
The current development setup guide is quite light:
At least, for me, it wasn't sufficient in order to get me started 😢
Maybe add:
and then make sure the code in the HEAD of master does compile with
lerna run build
.Cloning 131823a, I've got the following issues:
error TS5014: Failed to parse file 'tsconfig.json': Unexpected token ] in JSON at position 85.
which seems to be caused by trailing commas in arraysFixing those commas, the build then fails with:
$ lerna run build
And there you loose potential contributors 😐
The text was updated successfully, but these errors were encountered: