-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Build history #5371
Build history #5371
Conversation
Pull Request Test Coverage Report for Build 7786
💛 - Coveralls |
@kofj, VMware has approved your signed contributor license agreement. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments
public getManifest( | ||
repositoryName: string, | ||
tag: string, | ||
queryParams?: RequestQueryParams |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The optional 'queryParams' is not appended to the request. If caller pass parameters, it will be no effect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This optional is not used, should be removed.
@@ -13,7 +13,8 @@ import { | |||
clone | |||
} from '../utils'; | |||
import { ErrorHandler } from '../error-handler/index'; | |||
import { SystemSettingsComponent, VulnerabilityConfigComponent } from './index'; | |||
import { SystemSettingsComponent } from './system/system-settings.component'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason of changing the importing paths of these components
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it will warn: "Circular dependency" when running NG Live Development Server.
<div><clr-icon shape="play" size="20" class="tip-icon-low rotate-90"></clr-icon>{{lowCount}} {{'VULNERABILITY.SEVERITY.LOW' | translate }} {{'TAG.LEVEL_VULNERABILITIES' | translate }}</div> | ||
<div class="second-row"><clr-icon shape="help" size="18"></clr-icon>{{unknownCount}} {{'VULNERABILITY.SEVERITY.UNKNOWN' | translate }} {{'TAG.LEVEL_VULNERABILITIES' | translate }}</div> | ||
<div> | ||
<clr-icon shape="error" size="24" class="is-error"></clr-icon> {{highCount}} {{'VULNERABILITY.SEVERITY.HIGH' | translate }} {{'TAG.LEVEL_VULNERABILITIES' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems this part is only code formatting changes. But the format is a little inconsistent with the original style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is an error, will be fixed later.
import {JobLogViewerComponent} from "../job-log-viewer/job-log-viewer.component"; | ||
import {ChannelService} from "../channel/channel.service"; | ||
import {JobLogService, JobLogDefaultService} from "../service/job-log.service"; | ||
import { VULNERABILITY_DIRECTIVES } from "../vulnerability-scanning/index"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good format!
Let' holding for a while and merge it into master after @ninjadq and @zhoumeina complete the upgrading work of Clarity and Angular. Rebase will be required then! |
The related libraries of front-end UI are upgraded to new versions (Angular to 6.x, clarity to 0.12). Now we can start the merging work of this PR. Before doing that, I think you need to rebase your code to based on the new code base. Once you have done, please let us know. |
Hi @kofj – please rebase when you get a moment. Thanks! |
@steven-zou @ninjadq All checks have passed, review please. |
Pull Request Test Coverage Report for Build 9145
💛 - Coveralls |
@@ -0,0 +1,11 @@ | |||
<clr-datagrid [clrDgLoading]="loading"> | |||
<clr-dg-column style="width: 160px;">{{ 'TAG.CREATION' | translate }}</clr-dg-column> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this style="width: 160px;" to scss.
@kofj , I don't know why when I try to create some comments, it returns 405 not allowed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please modify according to the comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please modify according to the comment.
"RUNNING":"执行中", | ||
"ERROR":"错误", | ||
"STATUS": "状态", | ||
"START_TIME": "开始时间", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why need to modify this update_time to endtime. It should not be modified in this pr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change it according to comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add build histroy section and navbar to tag-detail component Add i18n items of build history Format code. Signed-off-by: Frank Kung <[email protected]>
Move style to scss file. Signed-off-by: Frank Kung <[email protected]>
Show build history info in the tag detail page as a default table panel.