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

split proxy module and refactor query/caller somewhat #215

Merged
merged 1 commit into from
May 12, 2020

Conversation

lyoshenka
Copy link
Contributor

No description provided.

app/proxy/proxy.go Outdated Show resolved Hide resolved
app/proxy/proxy.go Outdated Show resolved Hide resolved
app/proxy/proxy.go Outdated Show resolved Hide resolved
app/proxy/proxy.go Show resolved Hide resolved
@@ -1,17 +1,20 @@
package proxy
package rpcerrors
Copy link
Collaborator

Choose a reason for hiding this comment

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

After the renaming, it feels like EnsureAuthenticated does not belong in this package anymore.

app/proxy/proxy.go Show resolved Hide resolved
app/query/query.go Show resolved Hide resolved
app/query/cache/cache.go Show resolved Hide resolved
app/query/cache/middleware.go Show resolved Hide resolved
@@ -1,17 +1,18 @@
package proxy
package query
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure how I feel about the package being called query, which is just

a wrapper around client JSON-RPC query

The meat of the package logic seems to be in Caller but naming the package after it doesn't sound right either. Maybe proxy? 😂

@lbry-bot lbry-bot assigned lyoshenka and unassigned anbsky and lyoshenka May 11, 2020
@lyoshenka lyoshenka merged commit e7c2303 into master May 12, 2020
@lyoshenka lyoshenka deleted the split-proxy-module branch May 13, 2020 17:22
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.

None yet

2 participants