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.

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

Update some TODO_V2_IMPORT comments #1694

Merged
merged 2 commits into from
Dec 27, 2024
Merged

Conversation

bfinzer
Copy link
Contributor

@bfinzer bfinzer commented Dec 19, 2024

  • In this branch we're marking instances of TODO_V2_IMPORT with indications of what should be done to address the issue. All relate to properties that are present in V2 documents that have not yet been brought into the V3 document model. About 7 stories have been created based on these markers. But the remainder are all in codap-v2-types.ts and can be dealt with in situ. Here is a rough classification.
    • Properties that were present briefly in V2 documents but are no longer used. These can be ignored in the V3 import.
    • Properties that are also present in V3 and which refer to implemented features, but the carrying them over during the input process has not yet been brought about.
    • Properties present in the V3 model which do not yet have an implementation.
    • Properties not present in V3 but where the feature is present but defined differently. An example of this is transparency referring to the opacity of a graph point where this is stored as a separate property in V2 but is an intrinsic part of the fill of a point in V3.
    • Properties referring to a feature present in V2 but which is not yet implemented in V3.

@bfinzer bfinzer requested a review from kswenson December 19, 2024 17:26
@bfinzer bfinzer added the v3 CODAP v3 label Dec 19, 2024
Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.12%. Comparing base (1d68742) to head (b3f4536).
Report is 9 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (1d68742) and HEAD (b3f4536). Click for more details.

HEAD has 9 uploads less than BASE
Flag BASE (1d68742) HEAD (b3f4536)
cypress 10 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1694       +/-   ##
===========================================
- Coverage   85.86%   68.12%   -17.75%     
===========================================
  Files         607      607               
  Lines       31012    31012               
  Branches     8603     8594        -9     
===========================================
- Hits        26628    21126     -5502     
- Misses       4063     9276     +5213     
- Partials      321      610      +289     
Flag Coverage Δ
cypress 43.44% <ø> (-31.42%) ⬇️
jest 53.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

cypress bot commented Dec 19, 2024

codap-v3    Run #5667

Run Properties:  status check passed Passed #5667  •  git commit 72b1c32e6e: Merge pull request #1694
Project codap-v3
Branch Review main
Run status status check passed Passed #5667
Run duration 05m 58s
Commit git commit 72b1c32e6e: Merge pull request #1694
Committer William Finzer
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 36
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 220
View all changes introduced in this branch ↗︎

Copy link
Member

@kswenson kswenson left a comment

Choose a reason for hiding this comment

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

👍 LGTM -- just one potential typo

source?: string
// TODO_V2_IMPORT data context metadata importDate is not imported
// TODO_V2_IMPORT_CARRY_OVER G data context metadata importDate is not imported
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// TODO_V2_IMPORT_CARRY_OVER G data context metadata importDate is not imported
// TODO_V2_IMPORT_CARRY_OVER data context metadata importDate is not imported

…ations of what should be done to address the issue. All relate to properties that are present in V2 documents that have not yet been brought into the V3 document model. About 7 stories have been created based on these markers. But the remainder are all in codap-v2-types.ts and can be dealt with in situ. Here is a rough classification.

  * Properties that were present briefly in V2 documents but are no longer used. These can be ignored in the V3 import.
  * Properties that are also present in V3 and which refer to implemented features, but the carrying them over during the input process has not yet been brought about.
  * Properties present in the V3 model which do not yet have an implementation.
  * Properties not present in V3 but where the feature is present but defined differently. An example of this is `transparency` referring to the opacity of a graph point where this is stored as a separate property in V2 but is an intrinsic part of the `fill` of a point in V3.
  * Properties referring to a feature present in V2 but which is not yet implemented in V3.
@bfinzer bfinzer force-pushed the update-comments-v2-import-v3 branch from b3f4536 to 44b74a5 Compare December 27, 2024 16:58
@bfinzer bfinzer merged commit 72b1c32 into main Dec 27, 2024
7 of 8 checks passed
@bfinzer bfinzer deleted the update-comments-v2-import-v3 branch December 27, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3 CODAP v3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants