-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
feat(Baserow Node): Add uploading files & trigger node #10219
base: master
Are you sure you want to change the base?
Conversation
0801ed1
to
48c4736
Compare
Hey @cedricziel, Looking at the change it looks ok although you have updated the json file version whcih isn't needed, If you can revert that and add some tests for the new functionality we should eb able to get this merged. I will fix the PR title so it falls in line with our conventions. |
@Joffcom I'm struggling setting up some tests but I made the version change |
548491e
to
6fb9f06
Compare
@Joffcom the scope here has changed, can you give it a review please? |
4bf3b00
to
06665ae
Compare
Hey @cedricziel, Because the scope has changed so much and includes a new node we will not be accept this PR in it's current state. Ideally PRs should be kept small, We are also not adding new community nodes which means the trigger node you have created will need to be published to NPM. |
@Joffcom sorry, i am trying to understand how to proceed here. First of all thanks for letting me know - 2nd of all does that mean I'm supposed to publish new nodes to npm in a self-contained fashion? Is that documented somewhere? I'm a bit flabbergasted; how are you adding new services? |
@Joffcom I removed the trigger node as per our chat |
Summary
This PR adds:
Related Linear tickets, Github issues, and Community forum posts
Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)Screenshots
Result of a trigger node:
File upload via binary:
File Upload via url