update with new required callbacks

[dnsbty]

Ecto 3.2.0 came out a couple days ago, and it adds some new required callbacks to Ecto.Type. This PR adds those new callbacks and uses the same defaults as Ecto chose to.

Ecto changelog here: https://github.com/elixir-ecto/ecto/blob/master/CHANGELOG.md#enhancements Ecto defaults here: https://github.com/elixir-ecto/ecto/blob/master/lib/ecto/type.ex#L86-L87

This fixes #91

[PragTob]

Thanks for fixing this @dnsbty :tada: :green_heart:

A merge and release would be appreciated :dancer:

[lukaszsamson]

Still shows warnings during compilation

==> ecto_enum
Compiling 5 files (.ex)
warning: function embed_as/1 required by behaviour Ecto.Type is not implemented (in module EctoEnum.Typespec.TestModule.PGStatusEnum)
  lib/ecto_enum/typespec.ex:28: EctoEnum.Typespec.TestModule.PGStatusEnum (module)

warning: function equal?/2 required by behaviour Ecto.Type is not implemented (in module EctoEnum.Typespec.TestModule.PGStatusEnum)
  lib/ecto_enum/typespec.ex:28: EctoEnum.Typespec.TestModule.PGStatusEnum (module)

probably due to import instead of use in https://github.com/gjaldon/ecto_enum/blob/master/lib/ecto_enum/typespec.ex#L24 Anyway, I don't understand why test code is in lib instead of test/support

[joshuataylor]

Also still gives warnings, but if you do use Ecto.Type it'll work

edit:

Ah, ecto 2.x doesn't support that, only 3.x

[bernardd]

Anyway, I don't understand why test code is in lib instead of test/support

The reason is explained pretty clearly in the @moduledoc.

[joshuataylor]

You'll also need to add it to lib/ecto_enum/postgres/use.ex, and it'll then compile without warnings.

[dnsbty]

Thanks @joshuataylor just changed that

[mtarnovan]

Thank you all for the contributions. Would very much like to see this released.

[dnsbty]

@gjaldon anything I can do to help get this through quicker? We can run off my fork for now, but I'd rather stick with the official repo when possible.

[dnsbty]

I appreciate all the approvals, but I don't have write access. I think only @gjaldon can merge this.

[sheharyarn]

Hey @gjaldon, don't mean to rush you, but can you take a look?

[hickscorp]

Any ETA on when this could land in master with a bumped version @gjaldon ?

[PragTob]

Hm @gjaldon might not be doing OSS right now/might not have time for it. Aka no answer here so far. Someone might want to try and reach out in some other media (but politely) or we wait until he checks here again :)

[dnsbty]

I reached out via email yesterday

[joshprice]

For anyone wanting a quick fix before the release just use this mix.exs setup:

{:ecto_enum, github: "dnsbty/ecto_enum", ref: "a69a9dde72cf98497606f9603bd46e3924979145"},

Which will pick up exactly the fix in this PR: https://github.com/dnsbty/ecto_enum/commit/a69a9dde72cf98497606f9603bd46e3924979145

Then switch back once released.

[lpil]

Great work everyone!

It has been a while since this PR was opened. In the event that it does not get merged soon would anyone be interested in taking up maintenance and pushing to Hex under a new name? It would be nice to pull deps from GitHub.

[dnsbty]

@lpil I considered requesting the name from Hex under their dispute policy for abandoned packages so that everyone could keep pulling without changing the package name. Does that seem too drastic?

[lpil]

I think a change of package name is good from a security point of view because then people know that the ownership has changed and that they need to re-evaluate their trust of the library as a result. It is always possible that a malicious actor takes over an abandoned package so better for the switch to be explicit and opt in.

There has been some high profile attacks in the npm ecosystem recently where a package name changed hand like this and the new owner used the good reputation of the old owner to inject malicious code via what looked like a routine update.

[lpil]

Note I'm not trying to say that you are untrustworthy, just that I think that mutable package names is not ideal from a security point of view.

[ViljoenJG]

@lpil I considered requesting the name from Hex under their dispute policy for abandoned packages so that everyone could keep pulling without changing the package name. Does that seem too drastic?

I won't go as far as suggesting that this project has been abandoned. There was some relatively recent activity on this project (last 3 months). I do hope this gets merged soon, though.

[PragTob]

Yes, I hope the policy for abandoned packages would not trigger here yet.

[joshprice]

I agree that it's unfair to say this project is abandoned yet, since this a relatively minor issue with a clear, and safe workaround (https://github.com/gjaldon/ecto_enum/pull/92#issuecomment-542627004).

That said, how long is too long to wait for an official release before blessing a fork with a new maintainer? Has anyone heard from @gjaldon in other channels?

[dtip]

My company uses EctoEnum often - we would happily take up maintenance if there's demand for it.

Having multiple users with write permission would help avoid this kind of bottleneck in the future. Would anyone else here like to volunteer as an active maintainer?

[dnsbty]

We use it a fair amount at Podium as well, so I think we would be open to maintaining as well. It might be nice to have multiple maintainers so that we can avoid a similar issue in the future.

[OvermindDL1]

I agree that it's unfair to say this project is abandoned yet, since this a relatively minor issue with a clear, and safe workaround (#92 (comment)).

@joshprice That's not a workaround that works as then such a thing using it couldn't be published on hex or various other setups.

[salasrod]

I have a few projects that depend on this package, and with the silence I am one of the ones that would consider this abandoned, I've started a package transfer/dispute request with hex.pm.

[lpil]

@salasrod If you wish to maintain this package please do so under a new name rather than attempting to take ownership of this one. See previous discussion for rationale https://github.com/gjaldon/ecto_enum/pull/92#issuecomment-546222667

[salasrod]

I completely disagree with that rationale @lpil, I don't think people have time to read all dependencies they pull from packages, one of those might contain malware.

Tomorrow hypothetically @gjaldon could also have pushed a bitcoin mining tool in ecto_enum and you would have also been affected because you trust the code you are pulling from hex.

I don't think it's worth breaking dependencies and locks just because someone fears that a bad actor will come in and push malware into the repo, and most importantly the name fits exactly what the package does, and it's not worth making it harder for people to find the maintained version with a name change because we are hopeful that @gjaldon will return.

[lpil]

The difference between an existing and a new maintainer injecting malware is that we as users have already decided to trust the current a new maintainer and are aware of our risk position. If the package changes hands we need to reevaluate the package and whether it can be trusted. If the package changes hands silently then we are unable to reevaluate as we don't know we need to.

It may be that in your business you don't want or need this level of security and certainty, but that is not the case for everyone.

And as previously stated, there have been multiple fairly high profile instances of malicious code being injected into applications in the npm ecosystem in this way.

[gjaldon]

@dnsbty thank you for the contribution! very much appreciated 💜

As for the rest who have been waiting on the new release, thank you for the patience. I have been busy with work and life outside of open-source. Didn't realize the delay would cause quite a stir. Sorry for the trouble.

https://hex.pm/packages/ecto_enum/1.4.0 https://github.com/gjaldon/ecto_enum/releases/tag/v1.4.0

In the future, the best way to reach me is through Elixir-Slack channel where I am also an admin.

[lpil]

💜 thank you!

[PragTob]

@gjaldon Thank you very much! :green_heart: :rocket: :tada:

[Sgoettschkes]

Thank you very much!