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

bearriver: loopPre is defined twice #438

Closed
ivanperez-keera opened this issue Oct 12, 2024 · 5 comments
Closed

bearriver: loopPre is defined twice #438

ivanperez-keera opened this issue Oct 12, 2024 · 5 comments
Assignees
Milestone

Comments

@ivanperez-keera
Copy link
Owner

The function loopPre is defined twice: once in FRP.BearRiver, and once in FRP.BearRiver.Loop. Only one definition should be needed.

This prevents FRP.BearRiver from providing an API equivalent to Yampa's.

@ivanperez-keera
Copy link
Owner Author

ivanperez-keera commented Oct 12, 2024

My intuition here would be to just remove the one in FRP.BearRiver, or hide it in FRP.Yampa and re-export FRP.BearRiver.Loop from FRP.Yampa.

@pbrinkmeier
Copy link
Contributor

Hi, I'd like to work on this issue. The second approach seems less likely to break the API, so I would perhaps go with that?

@ivanperez-keera
Copy link
Owner Author

ivanperez-keera commented Oct 20, 2024

Great!

Yeah, let's go with the second approach. Can you please structure it this way?

  • Commit 1: Expose FRP.Yampa.Loop in FRP.Yampa and hide FRP.BearRiver.loopPre.
  • Commit 2: Document changes in CHANGELOG.

Always reference the issue with . Refs #438. at the end of the commit message summary, etc.

Do you think you can have it ready today? I need to have the next release ready tonight (EOB Pacific time).

@ivanperez-keera ivanperez-keera added this to the Dunai 0.13.2 milestone Oct 20, 2024
pbrinkmeier added a commit to pbrinkmeier/dunai that referenced this issue Oct 20, 2024
.

This commit removes the loopPre definition from FRP.BearRiver and
replaces it by a re-export from FRP.Yampa.Loop.
@pbrinkmeier
Copy link
Contributor

I applied the changes in #441. The duplicate definition of loopPre is removed and replaced by re-exports where required. Please check if I understood the task, then I will add a changelog entry.

@ivanperez-keera
Copy link
Owner Author

Thanks! I responded in the PR.

pbrinkmeier added a commit to pbrinkmeier/dunai that referenced this issue Oct 20, 2024
.

This commit removes the loopPre definition from FRP.BearRiver and
replaces it by a re-export from FRP.Yampa.Loop.
pbrinkmeier added a commit to pbrinkmeier/dunai that referenced this issue Oct 21, 2024
ivanperez-keera pushed a commit to pbrinkmeier/dunai that referenced this issue Oct 21, 2024
…keera#438.

The function loopPre is defined twice: once in FRP.BearRiver, and once
in FRP.BearRiver.Loop. Only one definition should be needed. This
prevents FRP.BearRiver from providing an API equivalent to Yampa's.

This commit modifies FRP.Yampa to hide the import of loopPre definition
from FRP.BearRiver and replaces it by a re-export of FRP.Yampa.Loop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants