Skip to content

Add Streams library #381

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Oct 16, 2018
Merged

Add Streams library #381

merged 10 commits into from
Oct 16, 2018

Conversation

@remo5000
Copy link
Contributor

@remo5000 remo5000 commented Oct 4, 2018 •
edited
Loading

Todo

  • Add streams* as well
  • Add to index.js
  • Add exports to reducers/externalLibraries.ts

@remo5000 remo5000 added the wip label Oct 4, 2018
@coveralls
Copy link

coveralls commented Oct 4, 2018 •
edited
Loading

Pull Request Test Coverage Report for Build 927

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 25.791%
Totals Coverage Status
Change from base Build 913: 0.02%
Covered Lines: 904
Relevant Lines: 2966
💛 - Coveralls

@remo5000
Remove streams test
820c033 
Integrating another test tool into this will take a non-trivial amount
of time, and can be done together with tests for the other libraries.
@remo5000
Add stream_max
cd38f79 
Also just copied the grader file for solidarity
@remo5000
Add file when loading website
17dcb75
@remo5000
Add streams library name
2b24b82
@remo5000
Add functions for streams
b704c45
indocomsoft
indocomsoft requested changes Oct 13, 2018
View reviewed changes
Copy link
Member

@indocomsoft indocomsoft left a comment

Choose a reason for hiding this comment

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

Lol the stream_take_max is not meant to be there -- sorry. Can just remove

Also, does this mean that backend needs to add STREAMS to external library type?

@remo5000 remo5000 removed the wip label Oct 13, 2018
indocomsoft
indocomsoft requested changes Oct 13, 2018
View reviewed changes
Copy link
Member

@indocomsoft indocomsoft left a comment

Choose a reason for hiding this comment

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

Just one more change and its good otherwise!

src/reducers/externalLibraries.ts Outdated
'integers_from',
'eval_stream',
'stream_ref',
'stream_take_max'
Copy link
Member

@indocomsoft indocomsoft Oct 13, 2018

Choose a reason for hiding this comment

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

Need to remove this as well.

Copy link
Contributor Author

@remo5000 remo5000 Oct 14, 2018

Choose a reason for hiding this comment

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

Good catch!

indocomsoft
indocomsoft approved these changes Oct 14, 2018
View reviewed changes
Copy link
Member

@indocomsoft indocomsoft left a comment

Choose a reason for hiding this comment

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

LGTM

@martin-henz
Copy link
Member

martin-henz commented Oct 14, 2018

Just in case you are not looking at telegram so often:

Just reading "Make streams be imported only when specified"
Actually streams should be part of Source §3, just like lists are. Streams are covered in Section 3.5, and therefore, we need to include them in Source §3.

@remo5000
Copy link
Contributor Author

remo5000 commented Oct 16, 2018

@ the merge commit

Alt Text

@remo5000 remo5000 merged commit 8a32c0e into master Oct 16, 2018
@thomastanck thomastanck deleted the streams branch December 14, 2018 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@indocomsoft indocomsoft indocomsoft approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@remo5000 @coveralls @martin-henz @indocomsoft