-
Notifications
You must be signed in to change notification settings - Fork 51
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.
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
Add artifact ID to KCL values that have them #5205
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
80507ed
to
1f16c85
Compare
// TODO: We're reusing the same UUID for multiple commands. This seems | ||
// like the artifact graph would never be able to find all the | ||
// responses. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it must be a bug. Separate issue though.
6469b06
to
35a285d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5205 +/- ##
==========================================
- Coverage 86.01% 86.01% -0.01%
==========================================
Files 92 92
Lines 33082 33093 +11
==========================================
+ Hits 28455 28464 +9
- Misses 4627 4629 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM this will help tease out that issue you raised with the reuse of IDs too
07201da
to
2bd5f8d
Compare
2bd5f8d
to
669a7c3
Compare
669a7c3
to
45053f1
Compare
45053f1
to
18d5980
Compare
Resolves #5186.
Weirdly, the
id
of Sketch and other core structs may get mutated. Sometimes theid
value is the same as the artifact ID, but not always. We track and expose the stable artifact ID for everything that has it. This isn't used yet, but it will be in a follow-up PR.