Skip to content

Document processing v2#669

Merged
Siegrift merged 8 commits intomainfrom
processing-v2
Nov 27, 2023
Merged

Document processing v2#669
Siegrift merged 8 commits intomainfrom
processing-v2

Conversation

@Siegrift
Copy link
Contributor

@Siegrift Siegrift commented Nov 18, 2023

Relates to api3dao/commons#27

Rationale

Read api3dao/commons#32

@Siegrift Siegrift requested review from dcroote and wkande November 18, 2023 17:38
@Siegrift Siegrift self-assigned this Nov 18, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Nov 18, 2023 •
edited
Loading

Visit the preview URL for this PR (updated for commit 288d014):

https://vitepress-docs--pr669-processing-v2-d3oh59pa.web.app

(expires Mon, 04 Dec 2023 08:10:42 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: e4c5db1acb62b36273dc03718b86834917dea599

Siegrift
Siegrift commented Nov 18, 2023
View reviewed changes
Siegrift
Siegrift commented Nov 18, 2023
View reviewed changes
Siegrift
Siegrift commented Nov 18, 2023
View reviewed changes
Siegrift
Siegrift commented Nov 18, 2023
View reviewed changes
@Siegrift Siegrift mentioned this pull request Nov 18, 2023
Closed
5 tasks
bbenligiray
bbenligiray reviewed Nov 20, 2023
View reviewed changes
@bbenligiray
Copy link
Member

bbenligiray commented Nov 20, 2023

One thing to note is that it's possible to specify an endpoint with no API call, so apiCallParameters, apiCallResponse, etc. are not suitable. A more generic naming would be more compatible with other kinds of operations (other than API calls) or more modular stuff like chained API calls, etc. that may be implemented in the future.

dcroote
dcroote reviewed Nov 21, 2023
View reviewed changes
@api3dao api3dao deleted a comment from dcroote Nov 21, 2023
@bbenligiray
Copy link
Member

bbenligiray commented Nov 22, 2023

I left a few small but imo somewhat important comments. If you respond and need me to respond, feel free to poke me with a DM.

@api3dao api3dao deleted a comment from dcroote Nov 23, 2023
@Siegrift Siegrift requested a review from dcroote November 25, 2023 07:33
@Siegrift
Copy link
Contributor Author

Siegrift commented Nov 25, 2023

I think the discussion is now done. @dcroote @wkande can you take a final look and approve?

dcroote
dcroote approved these changes Nov 27, 2023
View reviewed changes
Copy link
Collaborator

@dcroote dcroote left a comment

Choose a reason for hiding this comment

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

Only minor / nits, otherwise LGTM 👍

@Siegrift Siegrift merged commit daeff84 into main Nov 27, 2023
@Siegrift Siegrift deleted the processing-v2 branch November 27, 2023 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bbenligiray bbenligiray bbenligiray left review comments

@dcroote dcroote dcroote approved these changes

@wkande wkande Awaiting requested review from wkande

Assignees

@Siegrift Siegrift

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Siegrift @bbenligiray @dcroote