Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

Refactored literal type casting from flytepropeller #387

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hamersaw
Copy link
Contributor

TL;DR

To allow re-use of literal casting in other repositories this PR refactors the code out of flytepropeller.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

^^^

Tracking Issue

NA

Follow-up issue

NA

Signed-off-by: Daniel Rammer <daniel@union.ai>
@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #387 (121275b) into master (c7ae927) will increase coverage by 0.67%.
The diff coverage is 72.34%.

❗ Current head 121275b differs from pull request most recent head c6a4233. Consider uploading reports for the commit c6a4233 to get more accurate results

@@            Coverage Diff             @@
##           master     #387      +/-   ##
==========================================
+ Coverage   76.11%   76.78%   +0.67%     
==========================================
  Files          18       19       +1     
  Lines        1390     1469      +79     
==========================================
+ Hits         1058     1128      +70     
- Misses        280      281       +1     
- Partials       52       60       +8     
Flag Coverage Δ
unittests ?

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

Impacted Files Coverage Δ
clients/go/coreutils/typing.go 41.48% <41.48%> (ø)
clients/go/coreutils/casting.go 83.88% <83.88%> (ø)
clients/go/coreutils/literal.go 92.95% <87.27%> (ø)

... and 16 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Daniel Rammer <daniel@union.ai>
Signed-off-by: Daniel Rammer <daniel@union.ai>
@hamersaw hamersaw marked this pull request as ready for review April 19, 2023 15:23
}
}
// If t is an enum, it can be created from a string as Enums as just constrained String aliases
if t.literalType.GetEnumType() != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: use && with the conditions

// CastsFrom is a trivial type checker merely checks if types match exactly.
func (t trivialChecker) CastsFrom(upstreamType *core.LiteralType) bool {
// If upstream is an enum, it can be consumed as a string downstream
if upstreamType.GetEnumType() != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: use && with the conditions

@@ -0,0 +1,367 @@
package coreutils
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this module was just providing type checking for upstream and downstream nodes. I think the naming of this file is a bit weird. or can you add more comments to explain what exactly this module does?

@@ -0,0 +1,140 @@
package coreutils
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question here, the naming of the file does not describe what this module is doing

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants