Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove error for invalid dvc.yaml if no dvc.yaml file #3514

Merged
merged 7 commits into from
Mar 21, 2023
Merged

Remove error for invalid dvc.yaml if no dvc.yaml file #3514

merged 7 commits into from
Mar 21, 2023

Conversation

sroy3
Copy link
Contributor

@sroy3 sroy3 commented Mar 20, 2023 •
edited
Loading

Fixes #3511

Screen.Recording.2023-03-20.at.4.48.30.PM.mov

@sroy3 sroy3 added the bug Something isn't working label Mar 20, 2023
@sroy3 sroy3 self-assigned this Mar 20, 2023
@sroy3 sroy3 marked this pull request as ready for review March 20, 2023 20:42
@sroy3 sroy3 marked this pull request as draft March 20, 2023 20:44
@sroy3 sroy3 marked this pull request as ready for review March 20, 2023 20:49
mattseddon
mattseddon reviewed Mar 20, 2023
View reviewed changes
extension/src/test/suite/experiments/index.test.ts
}

expect(messageSpy).to.be.calledWithExactly(expectedTableData)
}).timeout(WEBVIEW_TEST_TIMEOUT)
Copy link
Member

Choose a reason for hiding this comment

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

[F] This one seems like a genuine cross-platform failure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, just me using resolves for the stub instead or returns. I had an await for it that I removed after the lint stage before pushing my changes, so local tests passed, but they were not doing what they should have and were changed before committing.

julieg18
julieg18 approved these changes Mar 21, 2023
View reviewed changes
Copy link
Contributor

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

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

I think it's unrelated to the pr, but shouldn't we be able to create a dvc.yaml if a user doesn't have one?

Screen.Recording.2023-03-21.at.7.36.27.AM.mov

@sroy3
Copy link
Contributor Author

sroy3 commented Mar 21, 2023

I think it's unrelated to the pr, but shouldn't we be able to create a dvc.yaml if a user doesn't have one?

Yes thanks, there was definitely a regression there.

@sroy3
Copy link
Contributor Author

sroy3 commented Mar 21, 2023

I think it's unrelated to the pr, but shouldn't we be able to create a dvc.yaml if a user doesn't have one?

Yes thanks, there was definitely a regression there.

Fixed

@sroy3 sroy3 enabled auto-merge (squash) March 21, 2023 14:13
@sroy3
Copy link
Contributor Author

sroy3 commented Mar 21, 2023 •
edited
Loading

ERROR: Coverage for lines (76.85%) does not meet global threshold (90%) Not sure how it got to that point.

Left a it.only in the code. I usually get a linting error for that, weird it wasn't caught this time.

@codeclimate
Copy link

codeclimate bot commented Mar 21, 2023

Code Climate has analyzed commit c50ded5 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 85.7% (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.

@sroy3 sroy3 merged commit cb40a6e into main Mar 21, 2023
@sroy3 sroy3 deleted the no-invalid-yaml branch March 21, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattseddon mattseddon mattseddon approved these changes

@julieg18 julieg18 julieg18 approved these changes

Assignees

@sroy3 sroy3

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Add a pipeline stage" doesn't work with no dvc.yaml
3 participants
@sroy3 @mattseddon @julieg18