Feature/agent1 adbist #973
Conversation
…tudio into feature/agent1
* added back button, run actions and overview accordion * refactoring * overview table complete * fixed the dropdown arrow for the overview section * added table for prev job list * fixed agent job result type * code cleaning and code review comments * fixed yarn.lock conflicts * added function for job history * changed vscode-languageclient version * changed yarn lock file * fixed yarn lock file * fixed yarn file * fixed css paths * added images to packaging step * fix resource path for packaging * added steps lists * fixed style and dimensions * fixed conflicts * implemented job list * added the Date and Status columns * update yarn files * merged feature/agent1 * added theme styling for light theme * changed yarn lock files * added method signatures for job history with DMP * added methods for job running * added job actions to sqlops
* added back button, run actions and overview accordion * refactoring * overview table complete * fixed the dropdown arrow for the overview section * added table for prev job list * fixed agent job result type * code cleaning and code review comments * fixed yarn.lock conflicts * added function for job history * changed vscode-languageclient version * changed yarn lock file * fixed yarn lock file * fixed yarn file * fixed css paths * added images to packaging step * fix resource path for packaging * added steps lists * fixed style and dimensions * fixed conflicts * implemented job list * added the Date and Status columns * update yarn files * merged feature/agent1 * added theme styling for light theme * changed yarn lock files * added method signatures for job history with DMP * added methods for job running * added job actions to sqlops * navigation works but is really slow to load data
…sqlopsstudio into feature/agent1-adbist
|
@abist can you target this to master branch now? I think we should start working off there now that the initial payload has been merged. |
extensions-modules/package.json
Outdated
| @@ -3,7 +3,7 @@ | |||
| "version": "0.1.0", | |||
| "description": "Shared modules for SQL Operations Studio extensions", | |||
| "dependencies": { | |||
| "dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#0.1.5", | |||
| "dataprotocol-client": "github:Microsoft/sqlops-dataprotocolclient#feature/agentDmp1", | |||
There was a problem hiding this comment.
please revert the change to this file
| </h1> | ||
| <table class="step-list"> | ||
| <tr class="step-row"> | ||
| <td height="30"> | ||
| <h3>Status:</h3> | ||
| </td> | ||
| <td> | ||
| <td height="30"> |
There was a problem hiding this comment.
should we create a style for this?
There was a problem hiding this comment.
cells have to be inline-styled
| @@ -111,22 +140,26 @@ export class JobHistoryComponent extends Disposable implements OnInit, OnDestroy | |||
| } | |||
| } | |||
|
|
|||
| private jobAction(action: string): void { | |||
| private jobAction(action: string, jobName): void { | |||
There was a problem hiding this comment.
can you add a type to jobName? In general I suggest we try to explicitly provide types when declaring variables. But we should certainly do this for function parameters.
src/sql/sqlops.d.ts
Outdated
| @@ -1061,13 +1061,13 @@ declare module 'sqlops' { | |||
| } | |||
|
|
|||
| export interface AgentJobHistoryInfo { | |||
| instanceID: number; | |||
| instanceId: number; | |||
There was a problem hiding this comment.
any reason we're switching ID to Id? In Typescript is the normal convention ID? In any case, I don't think it's a good idea to mix and match the two casing conventions within a single interface.
I think I may have been using Id since that is the C# convention. If you see that, please feel free to switch to the correct convention or note in code review. Thanks!
There was a problem hiding this comment.
@kburtram Every single variable or field with ID in the sqlops.d.ts file is in Pascal case (Id). I'm assuming it's because that's the way we receive it from the service.
Made a few changes, that we can decide to take or not. Here's the summary -
Also, since we don't have all the fields from the mock-up, can we get a different list of properties to show when the history page is visited? I've changed a couple on my own, just to get an opinion.