Add timestamps migration
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?
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:
- How the
NOT NULLconstraints were added to both the base migration and an "upgrade" migration, and the instructions in the changelog: https://github.com/tompave/fun_with_flags/commit/d25e4f00023bbce5867ba5b29dc1a7ab3ed3ddfe - How that extra upgrade migration was later removed: https://github.com/tompave/fun_with_flags/pull/161
I love how Oban handles migrations with versioning Maybe we can implement a similar migrator here, what do you think?
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. Thank you
Hey @tompave, I’ve tried multiple approaches, but I still can’t get a good result. We can use a compile-time flag to include timestamps in the schema during compilation, but at some point, we’ll need to introduce a breaking change anyway.
Why not release version 2.0 with the breaking changes? That seems like the simplest path forward. I can provide a migration guide.
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.
Just to make it easier to resume work for this later:
- This was attempted in https://github.com/tompave/fun_with_flags/pull/154.
- Plus a small fix in https://github.com/tompave/fun_with_flags/pull/159.
- Some of my notes in
69ea3a8, on the gotchas of adding columns with a default value in production. - Later reverted in https://github.com/tompave/fun_with_flags/pull/160.
hey @tompave , are there any blockers to release v 2.0? I also want to add timestamps to gate and flag, to show it in the FWF_UI, what do you think?
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! I completely understand that personal matters must come first. If there’s anything I can do to help test or collaborate on the v2 branch so that integration is smoother down the road, please let me know. Wishing you all the best, and thanks again for your time and support!