Skip to content

Conversation

@mattseddon
Copy link
Contributor

@mattseddon mattseddon commented Mar 21, 2023

3/4 main <- #3477 <- #3520 <- this <- #3532

Relates to #2277

Demo

Screen.Recording.2023-03-21.at.7.21.53.pm.mov

Note: The above shows that I need to add the "full update" logic for when the dvc.yaml is changed (will be done in #3532).

@mattseddon mattseddon added the product PR that affects product label Mar 21, 2023
@mattseddon mattseddon self-assigned this Mar 21, 2023
@mattseddon mattseddon changed the base branch from main to use-errors March 21, 2023 08:37
import { ErrorDecorationProvider } from '../../tree/errorDecorationProvider'
import { uniqueValues } from '../../util/array'

export class DecorationProvider extends ErrorDecorationProvider {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is identical to extension/src/repository/model/decorationProvider.ts and close to extension/src/experiments/model/decorationProvider.ts I can follow up.

@mattseddon mattseddon changed the base branch from use-errors to jump-plots-diff-bc March 22, 2023 00:40
@mattseddon mattseddon changed the base branch from jump-plots-diff-bc to use-errors March 22, 2023 00:40
@mattseddon mattseddon force-pushed the use-errors branch 3 times, most recently from d627859 to f2eda8c Compare March 22, 2023 02:50
@mattseddon mattseddon changed the base branch from use-errors to main March 22, 2023 03:05
@mattseddon mattseddon changed the base branch from main to use-errors March 22, 2023 03:05
@mattseddon
Copy link
Contributor Author

Note: there is a bug where SCM file decorations can run into the plots tree. I should be able to stop this from happening but it will require a workaround. E.g

image

@mattseddon mattseddon marked this pull request as ready for review March 22, 2023 19:09
treeItem.description = description
}
if (tooltip) {
treeItem.tooltip = tooltip
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of checking for the element before assigning it? If tooltip is undefined, treeItem.tooltip is undefined as well anyway, no? I'm asking because I'm probably missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right, there is no point. I'll update

@mattseddon mattseddon enabled auto-merge (squash) April 2, 2023 01:51
@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit 82fa5b4 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 2

The test coverage on the diff in this pull request is 90.5% (85% is the threshold).

This pull request will bring the total coverage in the repository to 95.1% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon merged commit 1e7f21c into main Apr 2, 2023
@mattseddon mattseddon deleted the show-tree-errors branch April 2, 2023 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

product PR that affects product

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants