dispatch-proxy icon indicating copy to clipboard operation
dispatch-proxy copied to clipboard

Want to contribute..

Open rlindsberg opened this issue 7 years ago • 14 comments

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

rlindsberg avatar Jun 04 '18 14:06 rlindsberg

@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?

alexkirsz avatar Jun 04 '18 16:06 alexkirsz

        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?
    
    

rlindsberg avatar Jun 05 '18 05:06 rlindsberg

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 avatar Jun 05 '18 07:06 alexkirsz

@rlindsberg I've added you as collaborator.

alexkirsz avatar Jun 05 '18 07:06 alexkirsz

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

rlindsberg avatar Jun 05 '18 08:06 rlindsberg

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! [email protected] prepublish: `grunt`
npm ERR! Exit status 6
npm ERR! 
npm ERR! Failed at the [email protected] 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?

rlindsberg avatar Jul 20 '18 12:07 rlindsberg

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.

alexkirsz avatar Jul 20 '18 12:07 alexkirsz

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!

rlindsberg avatar Jul 20 '18 13:07 rlindsberg

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.

alexkirsz avatar Jul 20 '18 13:07 alexkirsz

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.

rlindsberg avatar Jul 20 '18 13:07 rlindsberg

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:

  • VS Code as an editor. Atom or Sublime Text are good second options.
  • Prettier to automatically format your code. You can install the VS Code plugin which will do it automatically for you when saving a file.

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 avatar Jul 20 '18 13:07 alexkirsz

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 avatar Jul 20 '18 13:07 alexkirsz

@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 😁

alexkirsz avatar Aug 27 '18 11:08 alexkirsz

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

rlindsberg avatar Aug 27 '18 11:08 rlindsberg