search

tflori / angular-translator

translation module for angular
https://tflori.github.io/angular-translator/
MIT License
21 stars 6 forks source link

Update TranslateComponent.ts #49

Closed BorntraegerMarc closed 7 years ago

BorntraegerMarc commented 7 years ago

To fix (angular's default) tslint errors. These errors should definitely be fixed, because with tslint it is currently not possible to exclude folders when running ng lint. So these tslint errors cause our CI build to fail...

These were the errors:

ERROR: D:/projects/komed-health-web/node_modules/angular-translator/src/TranslateComponent.ts[10, 16]: Missing whitespace in interpolation; expecting {{ expr }}
ERROR: D:/projects/komed-health-web/node_modules/angular-translator/src/TranslateComponent.ts[9, 15]: The selector of the component "TranslateComponent" should be used as element (https://angular.io/st
yleguide#style-05-03)
ERROR: D:/projects/komed-health-web/node_modules/angular-translator/src/TranslateComponent.ts[11, 5]: Use the @Input property decorator instead of the inputs property (https://angular.io/styleguide#sty
le-05-12
BorntraegerMarc commented 7 years ago

@tflori could you please publish a new version, once this is resolved? :)

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 596757b816947b8f2ecfa1735c7393609f3b0289 on BorntraegerMarc:master into 7ac673c503d772d062b19ac6c88f2ec0516beae5 on tflori:master.

BorntraegerMarc commented 7 years ago

my suggestion is to change the failing tests / tslint config from travis. Because for most of the users the tslint will fail because they use the default one from angular...

tflori commented 7 years ago

I'm fine with changing the tslint configuration. So if build works I can release a new version..

Sent from my OnePlus ONEPLUS A3003 using FastHub

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 2592d8ee6cdecd8bf1243745171631ac1cf8eda8 on BorntraegerMarc:master into 7ac673c503d772d062b19ac6c88f2ec0516beae5 on tflori:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling f2273e129c37f849ad132a7e1382f2b1f3f3bb70 on BorntraegerMarc:master into 7ac673c503d772d062b19ac6c88f2ec0516beae5 on tflori:master.

BorntraegerMarc commented 7 years ago

@tflori One test still fails but I don't really know why. Could you help me out?

tflori commented 7 years ago

I don't understand why you changed TranslateParams but not tslint.json...

Is it necessary to have the @input declaration in front of the setters now?

tflori commented 7 years ago

I think the biggest problem will be the setting no-eval because without this the Translator.parse() method will throw an error. I think the problem is located in ng cli that does not exclude node_modules directory...

If you find a way without failing tests it would be great. I'm currently working on other projects and can't really help on this minor issue (from my point of view).

tflori commented 7 years ago

Is the only problem the selector of translate? Try moving the Input specification to @Component specification again. Maybe this helps.

I'm still wondering if ng lint checks the code of angular-translator?

Sent from my OnePlus ONEPLUS A3003 using FastHub

BorntraegerMarc commented 7 years ago

Thanks for the feedback. Changing the tslint.json wouldn't help because projects using the translator would have their own tslint.json and would override it. This is my source for tslint exclude not working: https://github.com/palantir/tslint/issues/73

Based on this doc, it seems like a bad practice: https://angular.io/guide/styleguide#style-05-03

But I figured out now that the files actually can be excluded from the angular config directly. So we don't need the tsconfig. I adapted this now and it works: https://github.com/mgechev/codelyzer/issues/268#issuecomment-298395031

Closing this PR although I recommend to adapt the project to the style guidelines at some point...