Integration of REST/API in client #4 #364
Conversation
bsekachev
commented
Mar 28, 2019
bsekachev
commented
- Many client fixes:
- Labels with white spaces and commas
- Defiant
- Splitting
- Grouping
- Repaired qunit tests for client
- Many codacy issues have been resolved
| @@ -27,6 +27,14 @@ | |||
| "airbnb", | |||
| ], | |||
| "rules": { | |||
| "no-new": [0], | |||
There was a problem hiding this comment.
Why do we need to disable rules? Why don't fix the code?
There was a problem hiding this comment.
@nmanovic
We can't fix the code. If we assign an expression 'new Class()' to any variable which is unusable, we got eslint error "unusable variable" . If we don't assign this expression to a variable, we got eslint error "new is not needed". If we remove new, we got syntax error and code doesn't work.
| @@ -27,6 +27,14 @@ | |||
| "airbnb", | |||
| ], | |||
| "rules": { | |||
| "no-new": [0], | |||
| "class-methods-use-this": [0], | |||
There was a problem hiding this comment.
Probably it is necessary to make methods as static if they don't use this?
There was a problem hiding this comment.
I thought about it. But there are many methods which is don't use "this" and if we do it static, they will become public available. I think it is not better way.
| @@ -27,6 +27,14 @@ | |||
| "airbnb", | |||
| ], | |||
| "rules": { | |||
| "no-new": [0], | |||
| "class-methods-use-this": [0], | |||
| "no-restricted-properties": [0, { | |||
There was a problem hiding this comment.
I believe I already merged such changes? Do you need to merge develop into your branch?
There was a problem hiding this comment.
I needed to do it in order to write unit tests. Probably small merge conflict will become here later, but I suppose it isn't big problem, to resolve it.
|
@bsekachev , in the past we decided to use "full" version of js libraries because it is easy to debug problems. Now you are adding "min" versions. What is the reason for that? Does our "minimizer" work bad? |
|
@nmanovic |
|
@bsekachev , if we don't have minimiser yet than it is OK. The right way is to use non-minimised version of all libraries and minimise them together with our code. Using minimised libraries is good only if you have a static page and don't have any pipeline to preprocess js. We have the pipeline but in the past we disabled minimiser because it worked incorrectly. |
|
@nmanovic |
Git can't check for this whitespace error, so get Pylint to do it instead.
