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

nodewrite encoder needs the instance type added #766

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

Jacob-Eliat-Eliat
Copy link
Contributor

@Jacob-Eliat-Eliat Jacob-Eliat-Eliat commented Jul 23, 2024

Obvious oversight =/ tests in cdp-spark-datasource will end up covering this, but maybe I could also write something in here if needed.

Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.48%. Comparing base (7de6e43) to head (90958b6).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #766   +/-   ##
=======================================
  Coverage   84.48%   84.48%           
=======================================
  Files          96       96           
  Lines        2772     2772           
  Branches      207      199    -8     
=======================================
  Hits         2342     2342           
  Misses        430      430           
Files Coverage Δ
.../sdk/scala/v1/fdm/instances/NodeOrEdgeCreate.scala 85.71% <100.00%> (ø)

@Jacob-Eliat-Eliat
Copy link
Contributor Author

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.48%. Comparing base (7de6e43) to head (90958b6).

Additional details and impacted files

Hmmmm

Copy link
Contributor

@dmivankov dmivankov left a comment

Choose a reason for hiding this comment

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

Assuming it's safe to insert it in any place, not necessarily making it the last field

@Jacob-Eliat-Eliat Jacob-Eliat-Eliat merged commit 326b2e9 into master Jul 23, 2024
9 checks passed
@Jacob-Eliat-Eliat Jacob-Eliat-Eliat deleted the nodeWriteEncoder-include-type branch July 23, 2024 12:35
@Jacob-Eliat-Eliat
Copy link
Contributor Author

Jacob-Eliat-Eliat commented Jul 23, 2024

Assuming it's safe to insert it in any place, not necessarily making it the last field

Should be, or we can complain to fdm ^^
What could be a problem is if nodeCreate fails in datasource if the view the instance is implementing doesn't have a type now that I think about it hmm, I'll check as quick as I can

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants