-
-
Notifications
You must be signed in to change notification settings - Fork 62
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: introduces a new resource "http" #1773
Conversation
ce0221a
to
8742e64
Compare
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
8742e64
to
01d21d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This plugin is awesome. I tested locally and it works great.
I also like the unit tests
I made a few suggestion to the plugin comment as I think they can be improve a bit but it's not a blocker
I am just wondering how we should communicate if it's better to fetch content using the http
plugin or the file
with the http protocol in the path
Feel free to resolve the conversations and the merge whenever you want
Co-authored-by: Olivier Vernin <olivier@vernin.me>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Thanks! I've taken your suggestions. If you see further improvement (on the comment particularly) don't hesitate to comment the PR, even if merged: i'll send a subsequent PR if need be.
Proposal, WDYT?
|
Self merging because:
from @olblak and I only changed comments since his last review |
Implements #268
This PR introduces a new resource of type
http
with source and condition capability.The initial UX proposal has been updated to ensure separation between source and condition are easier (see the E2E test).
Here is a "complete reference example":
Test
To test this pull request, you can run the following commands:
Additional Information