search

source-academy / stories-backend

Backend of Source Academy extension for Stories support.
0 stars 0 forks source link

Ensure migrations ordering is always preserved #32

Closed RichDom2185 closed 1 year ago

RichDom2185 commented 1 year ago

Adds a check in the CI workflow to ensure that all newly-created migration files are renamed accordingly with the appropriate timestamps to ensure a stable ordering of migrations in production.

github-actions[bot] commented 1 year ago

Coverage Status

coverage: 79.245%. remained the same when pulling 18dbdb6fe045ca5cbddcdde094e6a798e261412e on add-migrations-check-in-ci into 2e93a26b974d3ece341e0deaeef9174b26ae3890 on main.

RichDom2185 commented 1 year ago

For some reason, while 68e516394828c670a68f095d4e1bafd75e3143cd makes the regex safer, it breaks the script on macOS. @YaleChen299 do you think it's worth the tradeoff, especially considering the fact that the migration file names are autogenerated?

RichDom2185 commented 1 year ago

For some reason, while 68e5163 makes the regex safer, it breaks the script on macOS. @YaleChen299 do you think it's worth the tradeoff, especially considering the fact that the migration file names are autogenerated?

The issues lies documented here: https://riptutorial.com/sed/topic/9436/bsd-macos-sed-vs--gnu-sed-vs--the-posix-sed-specification.

Specifically,

Notably, POSIX specifies support only for basic regular expressions, which have many limitations (e.g., no support for | (alternation) at all, no direct support for + and ?) and different escaping requirements.

Caveat: GNU sed (without -r), does support |, + and \?, which is NOT POSIX-compliant; use --posix to disable (see below).

Which is why it works on our Ubuntu runners but not macOS for local. While they did say:

For more powerful regular expressions, use -E (rather than -r) to support EREs (extended regular expressions) (GNU sed doesn't document -E, but it does work there as an alias of -r...

They also said:

Sadly, this means that you won't be able to use several useful constructs, notably:

  • ...
  • back-references inside regular expressions (as opposed to the "back-references" to capture-group matches in the replacement string of s function calls), because BSD sed doesn't support them in extended regexes (but, curiously, does so in basic ones, where they are POSIX-mandated).

...and we need the \1 backreference for the numeric timestamp substitution.

Maybe one with more familiarity with regex can help with this...

RichDom2185 commented 1 year ago

Maybe one with more familiarity with regex can help with this...

...or we just avoid regex and sed entirely. With 18dbdb6fe045ca5cbddcdde094e6a798e261412e, this is now fixed 👍.