Skip to content
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

Add support for no downtime migrations #17

Merged
merged 8 commits into from
Jan 8, 2019
Merged

Conversation

arybczak
Copy link
Collaborator

No description provided.

@@ -1,6 +1,6 @@
cabal-version: 1.18
name: hpqtypes-extras
version: 1.6.3.0
version: 1.7.0.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also update CHANGELOG.md

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@trin-cz trin-cz self-assigned this Dec 14, 2018
Copy link
Contributor

@23Skidoo 23Skidoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, but I think there should be a bit more documentation. I also added a changelog and refactored the ValidationResult type - let me know what you think of this change.

, tblInitialSetup :: Maybe TableInitialSetup
tblName :: RawSQL () -- ^ Must be in lower case.
, tblVersion :: Int32
, tblAcceptedDbVersions :: [Int32]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be a Haddock comment explaining what this field is for.

@@ -11,16 +15,47 @@ import Prelude
data Check = Check {
chkName :: RawSQL ()
, chkCondition :: RawSQL ()
, chkValidated :: Bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any place where chkValidated is used (except initialisation), is it supposed to be useful to clients?

@@ -22,6 +25,7 @@ data ForeignKey = ForeignKey {
, fkOnDelete :: ForeignKeyAction
, fkDeferrable :: Bool
, fkDeferred :: Bool
, fkValidated :: Bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly to chkValidated, I don't see any actual uses of fkValidated. If clients are supposed to use this field, it should have a Haddock comment.

@@ -29,6 +31,7 @@ data TableIndex = TableIndex {
idxColumns :: [RawSQL ()]
, idxMethod :: IndexMethod
, idxUnique :: Bool
, idxValid :: Bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above: the library doesn't make use of idxValid, if the clients are supposed to, there should be a Haddock comment.

@@ -1,6 +1,6 @@
cabal-version: 1.18
name: hpqtypes-extras
version: 1.6.3.0
version: 1.7.0.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@arybczak
Copy link
Collaborator Author

arybczak commented Jan 7, 2019

I pushed a commit with explanations of new fields.

@23Skidoo
Copy link
Contributor

23Skidoo commented Jan 7, 2019

OK, I'll cut a release today.

@23Skidoo 23Skidoo merged commit 6eb1685 into master Jan 8, 2019
@23Skidoo 23Skidoo deleted the no-downtime-migrations branch January 8, 2019 08:30
@23Skidoo
Copy link
Contributor

23Skidoo commented Jan 8, 2019

Release 1.7.0.0 uploaded to Hackage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants