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

New blocktype FilePicker and abort mechanism (UAC-7-11 of #123) #136

Merged
merged 9 commits into from
Feb 20, 2023

Conversation

schlingling
Copy link
Contributor

@schlingling schlingling commented Feb 12, 2023

Introduces a new blocktype FilePicker and abort mechanism if an parent block emits a null value.

Part of RFC0002 mobility-extension.

Implements following UACs of #123:

  • [UAC-11] New blocktype FilePicker is implemented in std-extension..

I was not sure, how to register the new Block properly, so i will register it later.

@schlingling schlingling self-assigned this Feb 12, 2023
@schlingling schlingling changed the title [UAC-11] New blocktype FilePicker is implemented in std-extension (UAC-11 of #123) [WIP] New blocktype FilePicker is implemented in std-extension (UAC-11 of #123) Feb 12, 2023
@schlingling schlingling changed the title [WIP] New blocktype FilePicker is implemented in std-extension (UAC-11 of #123) New blocktype FilePicker is implemented in std-extension (UAC-11 of #123) Feb 12, 2023
@schlingling schlingling changed the title New blocktype FilePicker is implemented in std-extension (UAC-11 of #123) New blocktype FilePicker (UAC-11 of #123) Feb 12, 2023
Copy link
Contributor

@rhazn rhazn left a comment

Choose a reason for hiding this comment

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

I left one comment, once that is fixed this is fine to merge.

@georg-schwarz
Copy link
Member

georg-schwarz commented Feb 17, 2023

Please add documentation to the added BlockMetaInformation subclasses on the docs fields after a rebase. You can get inspiration from the existing blocks. You will get documentation when hovering over the FilePicker block type in the VSCode plugin out of the box ;-)

@schlingling
Copy link
Contributor Author

added doc to FilePicker @georg-schwarz. Thanks for already adding the docs to my HttpExtractor and ArchiveInterpreter :)

@schlingling schlingling changed the title New blocktype FilePicker (UAC-11 of #123) New blocktype FilePicker and abort mechanism (UAC-7-11 of #123) Feb 18, 2023
@schlingling
Copy link
Contributor Author

I left one comment, once that is fixed this is fine to merge.

@rhazn: I implemented the changes discussed in slack (return null value in filepicker)

Copy link
Contributor

@rhazn rhazn left a comment

Choose a reason for hiding this comment

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

Approved from my side, please wait for felix to have a look at the one comment I tagged him in.

{
code: blockExampleUsage,
description:
'A local SQLite file is created at the given path and filled with table data about cars.',
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a correct description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

damn, copy/paste-error, sorry! Will change it.

result.left.message,
result.left.diagnostic,
// Check, if parent emitted a value
if (inputValue != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an interesting question for @felix-oq as well I think.

I see:

  • Blocks could return combined types, e.g. FILE | NONE and we abort execution on NONE, this is the view "A block always has an output if it gets executed"
  • Blocks could return their output as FILE and also return nothing (in the typescript interpreters case null), this is the view "A block generates zero or more outputs on any input"

I personally think generating zero or more outputs for any input is the best. I know it from observables, it is very helpful later on because in a lot of cases you might want to generate multiple values for one input.

While the best way to implement that would be with the actual 0 to n outputs, I think a first iteration with 1 output or null for 0 outputs is fine.

So in summary, I like the usage of returning FILE or null here and would keep it but please, @felix-oq have a look and share your opinion as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense for me, will wait for felix, thanks for your input!

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, we can do it like that 👍

@schlingling schlingling merged commit 76febcf into main Feb 20, 2023
@schlingling schlingling deleted the feat-mobility-ext-uac-11 branch February 20, 2023 11:43
@github-actions github-actions bot locked and limited conversation to collaborators Feb 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants