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

Convert Direction from enum into a discriminated union #1618

Merged
merged 5 commits into from
Jul 8, 2021

Conversation

theunrepentantgeek
Copy link
Member

@theunrepentantgeek theunrepentantgeek commented Jul 4, 2021

What this PR does / why we need it:

Code that needed to make a decision based on the direction of conversion always needed to handle "other" values with a panic; converting the type into an interface allows us to ensure that only valid implementations are provided, simplifying consumption.

This type will be used more frequently in upcoming PRs to implement type conversion.

Harvested from PR #1533

Prerequisites

Special notes for your reviewer:

Let us take a moment and lament how SelectString() and SelectType would be a single method if Go had generics right now.

How does this PR make you feel:
gif

@codecov-commenter
Copy link

codecov-commenter commented Jul 4, 2021

Codecov Report

Merging #1618 (9308c06) into master (a9e9a0e) will decrease coverage by 0.03%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1618      +/-   ##
==========================================
- Coverage   64.92%   64.88%   -0.04%     
==========================================
  Files         202      203       +1     
  Lines       12608    12603       -5     
==========================================
- Hits         8186     8178       -8     
- Misses       3709     3711       +2     
- Partials      713      714       +1     
Impacted Files Coverage Δ
hack/generator/pkg/conversions/direction.go 40.00% <40.00%> (ø)
...or/pkg/conversions/property_assignment_function.go 80.40% <83.33%> (+1.26%) ⬆️
hack/generated/pkg/genruntime/resolver.go 81.35% <0.00%> (-3.39%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9e9a0e...9308c06. Read the comment docs.

@theunrepentantgeek theunrepentantgeek changed the base branch from master to feature/hub-function July 6, 2021 04:02
Base automatically changed from feature/hub-function to master July 7, 2021 02:53
Comment on lines +269 to +270
sourceType := fn.direction.SelectType(fn.otherDefinition.Type(), receiver.Type())
destinationType := fn.direction.SelectType(receiver.Type(), fn.otherDefinition.Type())
Copy link
Member

Choose a reason for hiding this comment

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

This bit is fiddly but I think still better than the original.

@theunrepentantgeek theunrepentantgeek merged commit 5e6b16b into master Jul 8, 2021
@theunrepentantgeek theunrepentantgeek deleted the improve/direction branch July 8, 2021 20:52
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.

3 participants