[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#32121] [PATCH 3/5] database: Add support for database upgrades.
From: |
Ludovic Courtès |
Subject: |
[bug#32121] [PATCH 3/5] database: Add support for database upgrades. |
Date: |
Fri, 13 Jul 2018 10:47:40 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Clément Lassieur <address@hidden> skribis:
> * Makefile.am: Copy SQL files into their data directory.
> * doc/cuirass.texi (Database schema): Document the change.
> * src/cuirass/database.scm (%package-sql-dir): New parameter.
> (db-load, db-get-version, db-set-version, get-target-version,
> get-upgrade-file, db-upgrade): New procedures.
> (db-init): Set version corresponding to the existing upgrade-n.sql files.
> (db-open): If database exists, upgrade it.
> * src/schema.sql: New file.
> * src/sql/upgrade-1.sql: New file.
Awesome!
What follows is nitpicking, but the patch otherwise LGTM!
For Makefile.am, please specify the new variables explicitly in the
commit log.
> dist_pkgdata_DATA = src/schema.sql
> +dist_sql_DATA = src/sql/upgrade-*.sql
This won’t really work; you have to explicitly list the files (or use
$(wildcard …), but I have a slight preference for an explicit list.)
> address@hidden SchemaVersion
^
Please add a space here…
> +This table keeps track of the schema version. During the initialization, the
… and here s/This table/The @code{SchemaVersion} table/
> +(define (db-get-version db)
Rather ‘db-schema-version’? Also, please consider adding docstrings to
top-level procedures.
> +(define (db-set-version db version)
Likewise: ‘db-set-schema-version’.
> +(define (get-target-version)
‘latest-db-schema-version’ maybe? :-)
> + (apply max
> + (map string->number
> + (map (cut match:substring <> 1)
> + (filter regexp-match?
> + (map (cut string-match
> + "^upgrade-([0-9]+)\\.sql$" <>)
> + (scandir (%package-sql-dir))))))))
I think you can write it along these lines:
(reduce max 0
(map (compose string->number (cut match:substring <> 1))
(filter-map (cut string-match …) (scandir …))))
> +(define (get-upgrade-file version)
‘schema-upgrade-file’?
> +(define (db-upgrade db)
> + (do-ec (:range version (db-get-version db) (get-target-version))
I would rather avoid SRFI-42, not just because I can’t parse it ;-), but
also to maintain consistency and make the code possibly more accessible.
In this case I think we could use a simple loop or (for-each … (iota n))
and that wouldn’t be bad.
WDYT?
Thank you!
Ludo’.
[bug#32121] [PATCH 4/5] database: Call a specification 'jobset' instead of 'project'., Clément Lassieur, 2018/07/10
- [bug#32121] [PATCH 4/5] database: Call a specification 'jobset' instead of 'project'., Ludovic Courtès, 2018/07/13
- [bug#32121] [PATCH 4/5] database: Call a specification 'jobset' instead of 'project'., Clément Lassieur, 2018/07/13
- [bug#32121] [PATCH 4/5] database: Call a specification 'jobset' instead of 'project'., Clément Lassieur, 2018/07/13
- [bug#32121] [PATCH 4/5] database: Call a specification 'jobset' instead of 'project'., Ludovic Courtès, 2018/07/13
- [bug#32121] [PATCH 4/5] database: Call a specification 'jobset' instead of 'project'., Clément Lassieur, 2018/07/14
[bug#32121] [PATCH 5/5] Add support for multiple inputs., Clément Lassieur, 2018/07/10