search

arjan / singleton

Global, supervised singleton processes for Elixir
MIT License
108 stars 19 forks source link

Let user run Singleton under their application's supervision tree #6

Closed ericdude4 closed 10 months ago

ericdude4 commented 4 years ago

Is there a reason why a user would want this running as its own OTP application? Also, running it as a separate application was always breaking my application as described in README was always breaking my app, but I think elixir runs the apps listed in deps() as applications now anyways.

This merge would require an increased major version, then I guess the README should be updated as well w/ new version.

arjan commented 4 years ago

If you would go this route, it only makes sense to do this when the supervisor in your own OTP app is named. And that would also mean that all Singleton.* calls would get an extra first argument, the name of the Singleton supervisor.

ericdude4 commented 4 years ago

@arjan I agree. I think this is the way to go moving forward though. This strategy of naming the supervisor based on your own OTP app seems to be the standard nowadays. Like I mentioned, make sure we increment the major version if you decide to merge this :)

I really like this package btw, works really well when paired with the :swarm package!

ericdude4 commented 4 years ago

Hey @arjan, have you had any time to review/think about this? I would love to stop referencing my fork in my deps in my production app :)

arjan commented 4 years ago

Yes! I think this is a good change. I will release 2.0.0 tomorrow if I get around to it.

ericdude4 commented 4 years ago

@arjan do you mind adding the hacktoberfest-accepted label to this pull request?

arjan commented 4 years ago

Sure, happy to!

ericdude4 commented 4 years ago

Hey @arjan, have you had some time to take a look?

nbw commented 2 years ago

I'm not sure if this will ever be merged, but if so then there are a few typos new docs: 'Sinlgeton' should probably be 'Singleton'

ericdude4 commented 2 years ago

@nbw I'm not sure why this never got merged, but please feel free to make any changes to this branch and please resolve the conflicts while you're at it :)

arjan commented 2 years ago

I am also not sure actually, I think it's a good change to structure the singleton like this. @nbw once conflicts are cleared i'll merge it!

ericdude4 commented 2 years ago

@arjan I fixed the typos that @nbw identified.

I also fixed the conflict, but wasn't sure what to do with : dynamic_supervisor_options().

MartinElvar commented 1 year ago

Any status on this @arjan ?

feld commented 1 year ago

Also looking for a status update

rauann commented 11 months ago

Any plan to release this change @arjan?

arjan commented 10 months ago

Finally merged! Thanks all for your patience.

arjan commented 10 months ago

I released 1.4.0 with this change.