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

refactor: rename internal references from Broadcaster to Gateway #3060

Merged
merged 3 commits into from
Jun 18, 2024

Conversation

rickstaa
Copy link
Contributor

@rickstaa rickstaa commented May 20, 2024

What does this pull request do? Explain your changes. (required)

This pull request updates internal references from 'Broadcaster' to 'Gateway' in accordance with the core team’s decision. For more details, refer to the discussion: Discord
Link
.

It follows up on #3056 as I noticed I forgot to rename some references.

Specific updates (required)

  • Renames some internal references of Broadcaster to Gateway in the livepeer.go and starter.go files.

How did you test each of these updates (required)

I checked the tests were successful and started a off-chain orchestrator and gateway.

Does this pull request close any open issues?

NO

Checklist:

This commit updates internal references from 'Broadcaster' to 'Gateway'
in accordance with the core team’s decision. For more details, refer to
the discussion: [Discord
Link](https://discord.com/channels/423160867534929930/1051963444598943784/1210356864643109004).
@rickstaa rickstaa requested a review from thomshutt May 20, 2024 09:39
Copy link

codecov bot commented May 20, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 57.40929%. Comparing base (e8f079e) to head (95418ef).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              master       #3060   +/-   ##
=============================================
  Coverage   57.40929%   57.40929%           
=============================================
  Files             92          92           
  Lines          15764       15764           
=============================================
  Hits            9050        9050           
  Misses          6111        6111           
  Partials         603         603           
Files Coverage Δ
cmd/livepeer/livepeer.go 50.98039% <100.00000%> (ø)
cmd/livepeer/starter/starter.go 7.90514% <0.00000%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8f079e...95418ef. Read the comment docs.

Files Coverage Δ
cmd/livepeer/livepeer.go 50.98039% <100.00000%> (ø)
cmd/livepeer/starter/starter.go 7.90514% <0.00000%> (ø)

@rickstaa
Copy link
Contributor Author

@victorges I noticed in #2981 hat you added the following line:

Since there's no goroutine or defer statement inside the loop that could cause a closure over the loop variable, we can safely remove the p := p line I think.

@victorges
Copy link
Member

Hey @rickstaa!
Even tho there's no explicit go or defer statements there, there is a closure on this anonymous function sent to that auto updater:

glog.Infof("Price: %v wei per pixel for broadcaster %v", price.FloatString(3), p.EthAddress)

And that updater itself might start a goroutine that eventually runs that function.

Do you think this could be made clear in some way?

@rickstaa
Copy link
Contributor Author

rickstaa commented Jun 5, 2024

Hey @rickstaa! Even tho there's no explicit go or defer statements there, there is a closure on this anonymous function sent to that auto updater:

glog.Infof("Price: %v wei per pixel for broadcaster %v", price.FloatString(3), p.EthAddress)

And that updater itself might start a goroutine that eventually runs that function.

Do you think this could be made clear in some way?

@victorges Ah, I think I had been programming for too long when I asked that question and overlooked the anonymous function 🤦🏻. Thanks for your explanation! I think in that case we are good to merge this 👍🏻.

@rickstaa rickstaa requested a review from victorges June 16, 2024 20:58
Copy link
Member

@victorges victorges left a comment

Choose a reason for hiding this comment

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

LGTM

@rickstaa rickstaa merged commit 20e81fb into master Jun 18, 2024
18 checks passed
@rickstaa rickstaa deleted the apply_small_broadcaster_renames branch June 18, 2024 10:41
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