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

Moving rpc to plugin #1290

Closed
wants to merge 1 commit into from

Conversation

melanke
Copy link

@melanke melanke commented Nov 26, 2019

⚠I am not being able to push because of this file: .github/workflows/dotnetcore.yml can you help me?!

We are moving the RpcServer to a plugin as discussed on this PR: neo-project/neo-modules#143

@erikzhang
Copy link
Member

image

We can't help. You need a rebase.

@melanke
Copy link
Author

melanke commented Nov 27, 2019

image

We can't help. You need a rebase.

done

@erikzhang
Copy link
Member

I don't understand why we need these ToPrimitive().

@melanke
Copy link
Author

melanke commented Nov 27, 2019

@erikzhang is to improve the usability of HttpServer controller, you will be able to receive and return primitive types instead of having to transform from/to JObject inside the RPC/Rest method.

But it is useful for many situations, not only on this case. I can't do this using extension methods because I need to overload the method.

@melanke melanke force-pushed the moving-rpc-to-plugin branch 4 times, most recently from 61dede1 to 14dd15b Compare November 28, 2019 13:21
@melanke
Copy link
Author

melanke commented Dec 4, 2019

Are this changes good? May I change all plugins depending on Rpc to use this HttpServer plugin?

items.ForEach(o => list.Add(o.ToPrimitive()));

return list.ToArray();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If these methods are only used in plugins, it is recommended to put them in plugin extension methods.

Copy link
Author

Choose a reason for hiding this comment

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

I can't do this using extension methods because I need to overload the method.

@Tommo-L
Copy link
Contributor

Tommo-L commented Dec 5, 2019

If we move RPC to plugin, I'm not sure will the Json module will be moved too?

@erikzhang
Copy link
Member

They should be moved too.

@shargon
Copy link
Member

shargon commented Dec 5, 2019

Json module now is used for SmartContracts. If we need in both sides, maybe we need neo-types project

@melanke
Copy link
Author

melanke commented Dec 6, 2019

But will neo-types be a dependency of neo?

@erikzhang erikzhang closed this Dec 7, 2019
@melanke
Copy link
Author

melanke commented Dec 9, 2019

@shargon so what is the point of moving out with neo-types if it will still be inside neo (as dependency)? I believe it will only bring difficulty working with one more project, don't you think?

@melanke
Copy link
Author

melanke commented Dec 9, 2019

@erikzhang sorry, I don't understand why this PR was closed, should I do something?

@erikzhang
Copy link
Member

This PR has not progressed for many days, and we need to move forward as soon as possible. So I removed RPC in #1340, and moved it to plugins: neo-project/neo-modules#160, neo-project/neo-modules#163

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants