alexkirsz / dispatch-proxy

Combine internet connections, increase your download speed
MIT License
3.47k stars 245 forks source link

Want to contribute.. #81

Open rlindsberg opened 6 years ago

rlindsberg commented 6 years ago

Hello Alexandre!

I'm a computer science student at Royal Institute of Technology in Sweden and want to make something awesome in the summer break. I love this proxy tool and want to contribute. Can you add me as collaborator in this project so that I can close issues and confirm merge requests? Of course I will carefully test the code before doing any merges. It would be great if we can collaborate together and make this project even better! Feel free to tell me what you think.

Sincerely, Roderick

alexkirsz commented 6 years ago

@rlindsberg Sounds great! I don't have much--if any--motivation to keep working on this project, but I would certainly appreciate some help supporting it in the long run.

I'll add you as collaborator. For now, if you want to merge any code into master/cut a release, you'll need to make a pull request for me to approve.

Is that ok with you?

rlindsberg commented 6 years ago
        Fantastic!! Yes I will consult you before making any changes to master. What do you think I start with? Maybe fixing some bugs and closing issues?
alexkirsz commented 6 years ago

This project could use some cleaning up w/ regards to issues/PRs :)

I'm not aware of any outstanding bugs, I think most issues stem from an incomprehension of exactly how this tool operates. A good first (but tedious) contribution would be sorting through the issues and compiling a FAQ. However, if you do find a bug (and I'm sure there are plenty), don't hesitate to send a fix.

I've wanted to rewrite this project in modern JavaScript for a long time to encourage contributions/forks. At the time when I wrote this, CoffeeScript was already getting slowly phased out of relevance by ES6. The codebase isn't very large, and I expect it would be even less so if written today.

A few issues are also feature requests that I haven't gotten around to implementing.

I'm probably forgetting a bunch of other ways in which you might want to contribute. Pick one :)

alexkirsz commented 6 years ago

@rlindsberg I've added you as collaborator.

rlindsberg commented 6 years ago

Thanks! I will start with a FAQ and tutorial with more details. I'm also trilled to rewrite the code in ES6.

rlindsberg commented 6 years ago

Hello! I was trying to rewrite this project in ES6 but encountered a problem.

I ran $npm install and got this error:

> grunt

Running "clean:0" (clean) task
>> 0 paths cleaned.

Running "coffee:compile" (coffee) task
>> src/dispatcher.coffee:5:5: error: Can't reference 'this' before calling super in derived class constructors
>>     @connectionsTotal = 0
>>     ^
>> In file: src/dispatcher.coffee
>> On line: 4
>>     @connectionsTotal = 0
>>     ^
Warning: CoffeeScript failed to compile. Use --force to continue.

Aborted due to warnings.
npm ERR! code ELIFECYCLE
npm ERR! errno 6
npm ERR! dispatch-proxy@0.1.4 prepublish: `grunt`
npm ERR! Exit status 6
npm ERR! 
npm ERR! Failed at the dispatch-proxy@0.1.4 prepublish script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

Can you confirm that this is a bug or did I configured it wrong?

alexkirsz commented 6 years ago

This error happens because I didn't fix the version of grunt-contrib-coffee in dependencies, which means that between the last time I compiled the code and today, the CoffeeScript language has probably changed to disallow some code pattern that was previously allowed. In this case, accessing the current instance in constructor before calling super.

If you rewrite this project in ES6, you won't need grunt or CoffeeScript anymore.

Grunt is the tool that automatically compiles the CoffeeScript files to javascript, as configured here.

By default, npm start runs grunt, as configured here (npm run prepuplish is automatically executed when running npm install at the root of the project).

I strongly recommend to setup babel for this project. I can help setting it up on a separate branch if you want.

rlindsberg commented 6 years ago

Thanks for pointing out! I add super connectionsTotal and everything worked again!

Oh really. Do you mean that I have to rewrite from scratch?

That would be lovely. Yes please!

alexkirsz commented 6 years ago

ES6 and CoffeeScript are two separate languages, so there is no other option than rewriting from scratch. However, translating from CoffeeScript to ES6 is fairly straightforward.

When I built this tool, I didn't have much experience in programming, and that shows quite clearly in some, if not most of the files. So there is still a lot of room for improvement, and some ES6 features will also allow for more concise and clearer code in some parts.

I'll setup a clean working environment on the next branch.

rlindsberg commented 6 years ago

Ok, I understand.

You definitely had learned a lot form building this tool! I don't have much experience in Node so I feel kinda lost.. It is a big project for a junior developer.

alexkirsz commented 6 years ago

If you're having trouble understanding something, don't hesitate to ask :) I'll review your PRs and let you know when you can do something better.

I think the best thing for now is to setup your working environment. For node/JS development, I recommend using:

I've pushed the boilerplate code for the rewrite on the next branch.

The code under the src/ directory is the source code of the app, written in ES6+ JavaScript (modern JS). When running npm install or npm run prepare or npm publish, it will automatically be compiled down to ES5 Javascript (for compatibility reasons) under the lib/ directory.

When developing, you can also run npm run watch in order to launch a process that will automatically recompile files on change, so you don't have to run npm install every time you change something and want to test it.

You can test the local tool by running node ./bin/dispatch.js <any> <other> <arg>. This will use the local dispatch and not the globally installed one.

Let me know if you have any other questions / if you need something else to get started.

alexkirsz commented 6 years ago

Also, remember that running node ./bin/dispatch.js will use the code under lib/, so don't forget to make sure it's up to date by running npm install or npm run watch.

alexkirsz commented 6 years ago

@rlindsberg Let me know if you need some help or pointers. I have some time as I'm on holidays for a week, and I've wanted to bring this repo up to date for a long time now 😁

rlindsberg commented 6 years ago

That's great!! I will let you know!