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

Implementing basic functions #3554

Merged
merged 10 commits into from
Jul 11, 2022

Conversation

JaroslavTulach
Copy link
Member

Pull Request Description

The language specification suggests to add five basic functions into the standard library. identity, flip, const, curry & uncurry.

Important Notes

The new functions are being added into existing Function.enso file. That may not be the best place, but it is not clear from the design spec how they are supposed to be imported. I can move them wherever needed.

There is a documentation provided for each of the functions, but I am not sure how to verify it is correct. Do we generate the documentation for stdlib somehow?

Checklist

Please include the following checklist in your PR:

  • [ x ] The documentation has been updated if necessary.
  • All code has been tested:
    • [ x ] Unit tests have been written where possible.

@JaroslavTulach JaroslavTulach requested a review from wdanilo June 29, 2022 03:33
@JaroslavTulach JaroslavTulach requested a review from 4e6 as a code owner June 29, 2022 03:33
@JaroslavTulach JaroslavTulach self-assigned this Jun 29, 2022
@JaroslavTulach JaroslavTulach force-pushed the wip/jtulach/ImplementingBasicFunctions_182331372 branch from 865bb36 to 84c4110 Compare June 29, 2022 06:50
Copy link
Member

@jdunkerley jdunkerley left a comment

Choose a reason for hiding this comment

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

Should make these member functions rather than extensions.

Copy link
Member

@jdunkerley jdunkerley left a comment

Choose a reason for hiding this comment

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

Just the indent (4 spaces) needed to get this working as member functions.

@wdanilo - I believe we want these to be global static functions we can just call?
I don't believe this is currently possible but should be once here work, statics and import completed. Is that the case?

Copy link
Member

@jdunkerley jdunkerley left a comment

Choose a reason for hiding this comment

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

These work in the current form but will need review once here. is finished.

@JaroslavTulach JaroslavTulach requested a review from hubertp June 30, 2022 09:06
@JaroslavTulach
Copy link
Member Author

Thanks for the review @jdunkerley, I followed your advice and modified the test 3b99fc4 as suggested. Then I tried:

enso$ git merge --squash 5fca901
enso$ sbt buildEngineDistribution
enso$ ./built-distribution/enso-engine-0.0.0-dev-linux-amd64/enso-0.0.0-dev/bin/enso --run test/Tests/src/Data/Function_Spec.enso 
identity:
    - identity on number
    - identity on text
    - identity on boolean
flip:
    - flip on number
    - flip on text
const:
    - const on number
curry:
    - curry on number list
uncurry:
    - uncurry on number list

That means, once @hubertp merges his changes, the PR here shall be also mergable as it is right now.

Copy link
Collaborator

@hubertp hubertp left a comment

Choose a reason for hiding this comment

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

Will try to merge once my work is done

@hubertp hubertp force-pushed the wip/jtulach/ImplementingBasicFunctions_182331372 branch from 3b99fc4 to 52f1052 Compare July 7, 2022 14:06
@JaroslavTulach JaroslavTulach added the CI: Ready to merge This PR is eligible for automatic merge label Jul 11, 2022
@mergify mergify bot merged commit 735053c into develop Jul 11, 2022
@mergify mergify bot deleted the wip/jtulach/ImplementingBasicFunctions_182331372 branch July 11, 2022 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants