Add phase concept to backend #42
Conversation
|
Could you try and just keep the relevant commit for this PR ? Currently the diff is quite hard to read because it includes the changes from #14 , and therefore it's really complex to see what the changes of this PR specifically are. |
|
You keep adding a lot of commits here (most of them unrelated to adding phases), which makes this pretty much impossible to review. For the future, it would be better to keep each PR constrained to what its title says, and open separate PRs for separate features/code additions (note that these PRs can depend on each other). In particular, adding new concepts/changing the concepts we already agreed on (e.g. your recent commit about I've cherry-picked your changes to the lib to a PR that I'll open this weekend that aims to add all of the DB tables to the library (and remove the then-redundant db init script, following a comment from @MagicalLay on the db init script PR). |
do not use sql keyword such as order as a column name
Co-authored-by: Guillaume Bury <guillaume.bury@gmail.com>
src/backend/phase.ml
Outdated
| let judge_artefact_descr = Ftw.Artefact.Descr.of_string phase.judge_artefact_description in | ||
| let head_judge_artefact_descr = Ftw.Artefact.Descr.of_string phase.head_judge_artefact_description in | ||
| let ranking_algorithm = Ftw.Ranking.Algorithm.of_string phase.ranking_algorithm in | ||
| let updated_p = Ftw.Phase.build id_p competition_p round_p judge_artefact_descr |
There was a problem hiding this comment.
Rather than building a phase with its ids (which is very error-prone as one could build a phase with the wrong id very easily), I'd rather the update function have the same type as the create function with an additional argument for the id of the phase.
There was a problem hiding this comment.
used exactly the same args as create. Phase is identified by competition_id and phase_id.
| [@@deriving yojson] | ||
| [@@deriving yojson, enum, show] | ||
|
|
||
| let all = List.filter_map of_enum (List.init (to_enum Jack_and_Jill + 1) Fun.id) |
There was a problem hiding this comment.
I'm not sure this is simpler than the current code, particularly for such a simple and small type...
|
tests are working now ! |
|
code repris par la PR #86 |
No description provided.