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

Partial value mapping with changes around extra type argument #48

Merged
merged 21 commits into from
Apr 2, 2020

Conversation

AttilaMihaly
Copy link
Member

closes #5 closes #46
#25

@AttilaMihaly AttilaMihaly requested review from reesh-a and a team April 1, 2020 00:44
@AttilaMihaly AttilaMihaly self-assigned this Apr 1, 2020
@DamianReeves
Copy link
Member

While we are at it, I think it makes sense to switch AccessControlled to a tagged union:

So I think:

type alias AccessControlled a =
   { access : Acccess
   , value: a
   }

type Access 
  = Public
  | Private

should become:

type AccessControlled a
  = Public a
  | Private a

@AttilaMihaly
Copy link
Member Author

While we are at it, I think it makes sense to switch AccessControlled to a tagged union:

So I think:

type alias AccessControlled a =
   { access : Acccess
   , value: a
   }

type Access 
  = Public
  | Private

should become:

type AccessControlled a
  = Public a
  | Private a

It was exactly what you suggest previously but it was very painful to work with so I changed it: c22e947#diff-aa5c2f490fc2bfa6d2d09f081b81242c

@DamianReeves
Copy link
Member

So I find it weird having the Types at the top level Morphir "namespace":

It feels like the following should be in an appropriate folder, otherwise they are just random toplevel things:

  • Graph.elm
  • JsonExtra.elm
  • Pattern.elm
  • ResultList.elm
  • Rewrite.elm
  • Rule.elm

Also some of these seem like the belong as part of the SDK.

Copy link
Contributor

@reesh-a reesh-a left a comment

Choose a reason for hiding this comment

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

Minor cosmetic change requests

@AttilaMihaly
Copy link
Member Author

So I find it weird having the Types at the top level Morphir "namespace":

It feels like the following should be in an appropriate folder, otherwise they are just random toplevel things:

  • Graph.elm
  • JsonExtra.elm
  • Pattern.elm
  • ResultList.elm
  • Rewrite.elm
  • Rule.elm

Also some of these seem like the belong as part of the SDK.

I'm open for anything. Can you suggest how would you change it specifically? My strategy is to go with the simplest approach and then refactor if needed rather to think about organizing the code upfront.

@AttilaMihaly AttilaMihaly merged commit 3cb978a into finos:master Apr 2, 2020
sfc-gh-lfallasavendano added a commit to Snowflake-Labs/morphir-elm that referenced this pull request Dec 13, 2023
…s#48)

* Add support for Snowpark generation of basic PatternMatch cases

The following cases are supported:

- Cases with literal values
- Cases with constructors of union types without parameters
sfc-gh-lfallasavendano added a commit to Snowflake-Labs/morphir-elm that referenced this pull request Dec 13, 2023
…s#48)

* Add support for Snowpark generation of basic PatternMatch cases

The following cases are supported:

- Cases with literal values
- Cases with constructors of union types without parameters
sfc-gh-lfallasavendano added a commit to Snowflake-Labs/morphir-elm that referenced this pull request Jan 2, 2024
…s#48)

* Add support for Snowpark generation of basic PatternMatch cases

The following cases are supported:

- Cases with literal values
- Cases with constructors of union types without parameters
sfc-gh-lfallasavendano added a commit to Snowflake-Labs/morphir-elm that referenced this pull request Jan 8, 2024
…s#48)

* Add support for Snowpark generation of basic PatternMatch cases

The following cases are supported:

- Cases with literal values
- Cases with constructors of union types without parameters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants