Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

docs: add docs in global.go #213

Merged
merged 4 commits into from
Feb 19, 2024
Merged

Conversation

miparnisari
Copy link
Contributor

While debugging #208 I did a bunch of renames that made me understand the code more easily.

@miparnisari miparnisari requested a review from Baliedge as a code owner January 22, 2024 03:00
global.go Outdated
@@ -65,29 +65,32 @@ func newGlobalManager(conf BehaviorConfig, instance *V1Instance) *globalManager
Help: "The count of requests queued up for global broadcast. This is only used for GetRateLimit requests using global behavior.",
}),
}
gm.runAsyncHits()
gm.runBroadcasts()
gm.toOwners()
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the confusion around the original names. Would prefer a more <Verb><Noun> nomenclature.

How about:

  • gm.runAsyncHits() -> gm.runBatches()

I think runBroadcasts() is appropriately named.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, for me it's still confusing, because i'm missing out on who is doing the action.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's implied the current peer is doing the action. Both of these start a goroutine that process these actions.

@miparnisari miparnisari changed the title chore: rename for more clarity docs: add docs in global.go Feb 19, 2024
@miparnisari miparnisari requested a review from Baliedge February 19, 2024 21:20
@Baliedge Baliedge merged commit 6f1e32a into mailgun:master Feb 19, 2024
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants