add additional items to the pool config tab#463
Conversation
| /** | ||
| * A reference to an application package to be deployed to a compute nodes | ||
| */ | ||
| export class CertificateReference { |
| public set pool(value: Pool) { | ||
| this._pool = value; | ||
| this.decorator = new PoolDecorator(this._pool); | ||
| this._refresh(this._pool); |
There was a problem hiding this comment.
Prefer doing this in the ngOnChanges, it is made for that
There was a problem hiding this comment.
Cool, though they are both set on change are they not? I would have thought the pool set method would be the perfect place for it?
According to these guys, if there is only a single input property then in the property setter is probably the best place for it. With many inputs, then ngOnChanges handles all changes at once, so would be probably better.
https://stackoverflow.com/questions/40577929/angular-2-setters-vs-ngonchanges
Adding in another event handler when not really required would seem to add an extra performance hit.
Just my thoughts.
| @Prop() public thumbprintAlgorithm: string; | ||
| @Prop() public storeLocation: string; | ||
| @Prop() public storeName: string; | ||
| @Prop() public visibility: string[]; |
There was a problem hiding this comment.
Needs to be ListProp here
@ListProp(String) public visibility: List<string> = List([]);
Don't forget to set the default value to be an empty list too that way we never have to check for null for list
| @@ -0,0 +1,12 @@ | |||
| import { Record } from "immutable"; | |||
|
|
|||
| const NetworkConfigurationRecord = Record({ | |||
| export class StartTaskInfo extends Record<StartTaskInfoAttributes> { | ||
| @Prop() public state: TaskState; | ||
| @Prop() public startTime?: Date; | ||
| @Prop() public endTime?: Date; |
There was a problem hiding this comment.
don't make some attribute here optional ? only in the interface for the attributes
| public static computeRuntime(startTime?: Date, endTime?: Date): string { | ||
| if (!startTime) { | ||
| return null; | ||
| } else { |
… picked up any more for some unknown reason.
3 participants
Fix #462