Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dependency: update to angular 11 #308

Merged
merged 1 commit into from
Dec 2, 2020
Merged

dependency: update to angular 11 #308

merged 1 commit into from
Dec 2, 2020

Conversation

jma
Copy link
Contributor

@jma jma commented Nov 25, 2020

Co-Authored-by: Johnny Mariéthoz Johnny.Mariethoz@rero.ch

Why are you opening this PR?

  • Angular 8 is end of life.

How to test?

  • What command should I have to run to test your PR?
  • What should I test through the UI?

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

Comment on lines 141 to 143
return Array.isArray(this._routerLink) && this._routerLink.length > 0
? true
: false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note : We could simplify with only return (Array.isArray(this._routerLink) && this._routerLink.length > 0);

) { }

/**
* hook OnInit
*/
ngOnInit() {
const initialState: any = this._modalService.config.initialState;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I learn something 👍

if (!locale) {
locale = this._translateService.currentLanguage;
}

return super.transform(value, format, timezone, locale);
return super.transform(value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why removing format, timezone, locale from parameters ? There seem still valid in Angular11

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@iGormilhit iGormilhit added this to the v0.15.0 milestone Nov 26, 2020
@iGormilhit iGormilhit added the dev: dependencies Pull requests that update a dependency file label Nov 26, 2020
@@ -17,7 +17,7 @@

/* tslint:disable:no-unused-variable */

import { TestBed, async, inject } from '@angular/core/testing';
import { TestBed, inject, waitForAsync } from '@angular/core/testing';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new import is not used

@@ -40,6 +40,9 @@ import { isEmpty, orderedJsonSchema, removeEmptyValues } from './utils';
import { LoadTemplateFormComponent } from './widgets/load-template-form/load-template-form.component';
import { SaveTemplateFormComponent } from './widgets/save-template-form/save-template-form.component';

export interface JSONSchema7 extends JSONSchema7Base {
form: any;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a space after the interface, as this is not linked to component. Or better in a specific file.

if (!locale) {
locale = this._translateService.currentLanguage;
}

return super.transform(value, format, timezone, locale);
return super.transform(value);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@jma jma force-pushed the maj-angular-11 branch 2 times, most recently from 401a731 to 8e77c2e Compare November 30, 2020 13:00
Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>
@sebdeleze sebdeleze merged commit fafc6ea into rero:dev Dec 2, 2020
@jma jma deleted the maj-angular-11 branch December 13, 2021 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev: dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants