Watch for dvc.yaml changes for manually added stages #3365
Conversation
| public async changeHasConfig(update?: boolean) { | ||
| const stages = await this.hasStages() | ||
| this.hasValidDvcYaml = stages !== undefined | ||
| this.hasConfig = !!stages |
There was a problem hiding this comment.
[Q] Is there any way that this.hasValidDvcYaml isn't !this.hasStages? Is this.hasValidDvcYaml === !this.hasConfig always true? What values of stages mean that we need both variables? Can we refactor to make it more obvious? (my brain is tiny)
There was a problem hiding this comment.
dvc stage list will return '' if no pipelines were found. It will return an error ('./dvc.yaml' validation failed.) if the dvc.yaml file is broken. the listStages command will return the result, but it has a catch clause that will return undefined.
In other words, hasInvalidDvcYaml = hasStages() === undefined and doesNotHaveConfig = hasStages() === ''.
We cannot test accurately for hasConfig if hasValidDvcYaml is true, but I chose to set it to false because it shows the message at the bottom of the table.
| } | ||
|
|
||
| export const AddStage: React.FC<AddStageProps> = ({ hasValidDvcYaml }) => ( | ||
| <div className={styles.addConfigButton} data-testid="aaa"> |
There was a problem hiding this comment.
Should we name this id to be a more clear name? Though it looks like it isn't being used anywhere 🤔
There was a problem hiding this comment.
Nice catch It was for quick testing (had a lot of trouble testing the shadow DOM button), but I forgot to remove it after.
|
Code Climate has analyzed commit 78866e1 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.0% (0.0% change). View more on Code Climate. |
Screen.Recording.2023-02-27.at.5.35.05.PM.mov