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

fix(setup): merge configuration inside harpoon.setup() #557

Open
wants to merge 2 commits into
base: harpoon2
Choose a base branch
from

Conversation

kimabrandt-flx
Copy link

Instead of initializing the users configuration inside harpoon:new(), do it when harpoon:setup({}) is called. This fixes the issue, when using the settings.key-function for the setup-call.

This change brakes one of the testies :\

Instead of initializing the users configuration inside `harpoon:new()`,
do it when `harpoon:setup({})` is called. This fixes the issue, when
using the `settings.key`-function for the setup-call.
@ThePrimeagen
Copy link
Owner

Great, I can take this from here

In order to minimize this PR, some changes have been reverted. The issue
is not about merging the config, but initializing the data.
@kimabrandt-flx
Copy link
Author

This reverts some changes, to make it more minimal, which also lets the tests pass now.
This duplicates the commit rndev-io@b8cc0e0 and #565 issue.

@rndev-io
Copy link

@ThePrimeagen Hi!
Can you merge pls this pr, or rndev-io@b8cc0e0

@joalof
Copy link

joalof commented Jul 16, 2024

Currently running into issues with this as well, would love to see this merged 🙏 :)

@mcDevnagh
Copy link

+1

metalinspired added a commit to metalinspired/harpoon that referenced this pull request Nov 1, 2024
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.

5 participants