-
Notifications
You must be signed in to change notification settings - Fork 84
Add timestamps migration #195
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
base: master
Are you sure you want to change the base?
Conversation
Thank you for the work and the PR! Can you please test this version of the code with and without the migration applied? That's necessary to write upgrade instructions. Ideally, this should be a non-breaking change if we want to ship it in the 1.x line. |
It would also be good to modify the current migration so that users can copy just one migration file when installing the package for the first time. For example, look at:
Maybe in v2.0, but I think it should not be in this PR. |
make it non breaking might be hard. I'll try something. |
Hey @tompave, Why not release version 2.0 with the breaking changes? That seems like the simplest path forward. Most likely, existing users will just add a migration with timestamps, and new users will include it during table creation. |
Yes, I was thinking that adding this to v2.0 would be best. In that case it's ok if it's a breaking change. Please leave the PR open for now. I'll tag it with the v2.0 label. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very straightforward, but very long awaited changes. Thanks Vitaly!
Just to make it easier to resume work for this later:
|
hey @tompave , are there any blockers to release v 2.0? defmodule FunWithFlags.Flag do
defstruct name: nil, gates: [], inserted_at: nil, updated_at: nil
defmodule FunWithFlags.Gate do
defstruct [:type, :for, :enabled, :inserted_at, :updated_at]
defmodule FunWithFlags.Store.Serializer.Ecto do
@moduledoc false
alias FunWithFlags.Flag
alias FunWithFlags.Gate
alias FunWithFlags.Store.Persistent.Ecto.Record
@spec deserialize_flag(atom() | String.t(), list()) :: Flag.t()
def deserialize_flag(name, []), do: Flag.new(to_atom(name), [])
def deserialize_flag(name, list) when is_list(list) do
gates =
list
|> Enum.sort_by(& &1.gate_type)
|> Enum.map(&deserialize_gate(to_string(name), &1))
|> Enum.reject(&(!&1))
inserted_at = gates |> Enum.map(& &1.inserted_at) |> Enum.min()
updated_at = gates |> Enum.map(& &1.updated_at) |> Enum.max()
Flag.new(to_atom(name), gates, inserted_at, updated_at)
end
|
hey @tompave , sorry for disturbing. I really want to merge timestamps 😄 |
Hey, because of personal reasons it's unlikely I'll be able to pay much attention to this in the short term. If timestamps are critical for you please use your fork. If you want, you can work against the v2 branch to make integration easier later. |
Hey! |
Hey @tompave, I’ve created the first PR containing just migration timestamps.
I love how Oban handles migrations with versioning:
https://github.com/oban-bg/oban/blob/main/lib/oban/migration.ex
Maybe we can implement a similar migrator here, what do you think?