Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Update mdbook to reflect "call" method changes #1203

Merged
merged 7 commits into from
May 1, 2019
Merged

Conversation

maackle
Copy link
Collaborator

@maackle maackle commented Apr 2, 2019

Partially complete update to mdbook to reflect changes to call RPC method. Still needs to be updated in two subsections.

I also propose:

  • we rename "params" to "args" or "arguments", to avoid nested the nested "params".
  • we allow both "params" and "args" in the "call" method handler, deprecating "params".
  • then we update the documentation to use "args".

Also, params is a misnomer here anyway. According to generally accepted notions, "parameters" are the placeholders you put in a function definition, and "arguments" are the actual values you pass when invoking a function, and the latter is what we're doing here.

This is what I'm talking about

{
    "jsonrpc": "2.0",
    "id": "0",
    "method": "call",
    "params": {  // <--------- from JSON-RPC spec
        "instance_id": "test-instance",
        "zome": "blog",
        "function": "create_blog",
        "params": {  // <---------- proposing we change to "args"
            "blog": {
                "content": "sample content"
            } 
        }
    }
}
  • my changes to the code affect some exposed aspect of the developer experience, and I have added an item to relevant 'Added, Fixed, Changed, Removed, Deprecated, or Security' heading under the 'Unreleased' heading of the CHANGELOG, with the format - summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)

@maackle maackle added help wanted Extra attention is needed good first issue Good for newcomers documentation labels Apr 2, 2019
@Connoropolous
Copy link
Collaborator

@maackle I also really disliked the params -> params
so I am very in favor of updating to using a key called args 👍

@zippy
Copy link
Member

zippy commented Apr 3, 2019

I'm ok with doing this update, but if we are going to do it, lets convert this PR/branch into not just a documentation change, but do the work here too. Also we should do it soon. Someone wanna take this on and add into the tree?

@maackle
Copy link
Collaborator Author

maackle commented Apr 3, 2019

I can definitely take it on next week, and maybe this week.

We can bundle up #1191 with this work as well.

@maackle
Copy link
Collaborator Author

maackle commented Apr 3, 2019

But, @zippy if by "soon" you mean this week, maybe someone else can take it on

@zippy
Copy link
Member

zippy commented Apr 3, 2019

Hey @dymayday how about you take these on!?

@dymayday
Copy link
Contributor

dymayday commented Apr 4, 2019

Sure! I will be more than happy to do it as soon as I'm done with the guidebook, I don't want to mess with my first PR...

@dymayday dymayday mentioned this pull request Apr 16, 2019
2 tasks
@Connoropolous Connoropolous changed the title WIP: update mdbook to reflect "call" method changes Update mdbook to reflect "call" method changes May 1, 2019
@Connoropolous Connoropolous merged commit 4f4a0f0 into develop May 1, 2019
@zippy zippy deleted the mdbook-update branch October 4, 2019 18:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants