Implementing classes using the new ES6 syntax#3069
Implementing classes using the new ES6 syntax#3069leonardo-silva wants to merge 4 commits intoexpressjs:5.0from
Conversation
|
I don't think this is compatible with Node.js 0.10, which is what your PR is targeting. I'm not sure of the value, but it hasn't been decided yet when we would drop support for versions of Node.js that does not support this syntax. Would it be possible to at least change your PR to target the 5.0 branch? |
|
I see. I've just changed the target branch to 5.0. |
|
Great! When you get the chance, please try to resolve the merge conflicts with the 5.0 branch so we at least have the option to merge :) as right now we cannot merge it even if we desired to. |
examples/static-files/index.js
Outdated
| console.log(' GET /hello.txt'); | ||
| console.log(' GET /js/app.js'); | ||
| console.log(' GET /css/style.css'); | ||
| console.log(' GET /css/style.css'); No newline at end of file |
There was a problem hiding this comment.
Please don't remove newline from the end of files :)
|
These merge conflicts are due to the change of branch target (master -> 5.0). I think the best thing to do now is to close this PR and open another comparing branches (5.0 -> 5.0). |
4 participants
Hi,
In my PhD research, I am using express in some experiments.
As a result, I detected the existence of at least 3 main "classes" in the current codebase (View, Route, Layer).
So, I decided to rewrite this code to follow the new syntax for classes provided by ECMAScript 2016, which I think is much more readable. All changes are just syntactical. I was also very careful to preserve the code style.
Finally, all tests are green in the migrated code.
Do you think it is worth to integrate these changes to the project?
Thank you