Add top-level plots wizard #4586
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
julieg18
changed the title
extension/src/fileSystem/index.ts
Outdated
| const plotYaml = yaml.stringify({ plots: [{ [plotName]: plot }] }).split('\n') | ||
|
|
||
| // TBD this only works correctly for yaml with 2 space indent and no plots | ||
| // will adjust for other possibilities in another pr |
There was a problem hiding this comment.
Will get taken care of in #4628 (Almost done, found a bug while finishing up tests. Will finish it tomorrow).
There was a problem hiding this comment.
[C] Let's not leave comments TODO in the code/merge before fixing obvious/known issue. It would have been enough to add a do not merge label to this PR and leave a comment linking to #4628 instead.
There was a problem hiding this comment.
Makes sense! Removed the unneeded comments.
extension/src/pipeline/util.ts
Outdated
| export const pickPlotConfiguration = async (): Promise< | ||
| PlotConfigData | undefined | ||
| > => { | ||
| // TBD data file validation will be in next pr |
There was a problem hiding this comment.
Taken care of in #4627.
| @@ -423,6 +423,12 @@ | |||
| "category": "DVC", | |||
| "icon": "$(symbol-class)" | |||
| }, | |||
| { | |||
| "title": "Add Plot", | |||
There was a problem hiding this comment.
Maybe "Add Top-Level Plot" to better differentiate between "Add Custom Plot"?
| @@ -903,6 +909,10 @@ | |||
| "command": "dvc.showPipelineDAG", | |||
| "when": "dvc.commands.available && dvc.project.available" | |||
| }, | |||
| { | |||
| "command": "dvc.addTopLevelPlot", | |||
| "when": "dvc.commands.available && dvc.project.available" | |||
There was a problem hiding this comment.
Is currently available in the command pallette.
extension/src/fileSystem/index.ts
Outdated
| const plotYaml = yaml.stringify({ plots: [{ [plotName]: plot }] }).split('\n') | ||
|
|
||
| // TBD this only works correctly for yaml with 2 space indent and no plots | ||
| // will adjust for other possibilities in another pr |
There was a problem hiding this comment.
[C] Let's not leave comments TODO in the code/merge before fixing obvious/known issue. It would have been enough to add a do not merge label to this PR and leave a comment linking to #4628 instead.
| return | ||
| } | ||
|
|
||
| return { ...templateAndFields, dataFile: file } |
There was a problem hiding this comment.
Avoid too many return statements within this function.
|
Code Climate has analyzed commit 0a0960f and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 91.9% (85% is the threshold). This pull request will bring the total coverage in the repository to 95.0% (0.0% change). View more on Code Climate. |
1/3
main<= this <= #4627 <= #4628Demo
Screen.Recording.2023-09-06.at.6.30.47.PM.mov
Fixes #3539