code review and response

doc/bugs/Old_dependencies_are_causing_pain_with_NixOS_18.09/comment_1_6378778d1424c704b827dfb06650074a._comment

@@ -0,0 +1,19 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2018-11-05T17:38:56Z"
+ content="""
+I'd kind of like to switch to using the Amazonka package for S3;
+it may be better maintained and there are several modern S3 features
+that the haskell aws library is lacking support for.
+Patches in that direction would be great.
+
+But I see that this is actually a conflict between dependencies of aws and
+equelito, so [[todo/Retire_Esqueleto_as_a_dependency]] would fix the
+immediate problem.
+
+(I've been doing some work to keep aws current with new versions of its
+dependencies, eg 
+https://github.com/aristidb/aws/commit/6fa37706d13133d0b5c406bca2f304d1e567d028
+since it's otherwise been lagging with such updates.)
+"""]]

doc/todo/Retire_Esqueleto_as_a_dependency/comment_1_b022e0e0a7711f306036b457a34144e4._comment

@@ -0,0 +1,58 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2018-11-05T17:34:43Z"
+ content="""
+Wow, this is impressive work!
+
+I was aware of the maintenance difficulties, but 
+had not realized it would be feasible to remove Esqueleto.
+
+That said, is it fully abandoned or only hit a rough patch?
+Last I heard there was at least desire to keep it going, and indeed 
+esqueleto's git master will build with the current haskell ecosystem. 
+But they wanted to make some other "correctness" changes before release.
+It felt to me like perhaps tying those changes together with the Persistent
+upgrade was not a great decision, but hard to tell.
+<https://github.com/bitemyapp/esqueleto/issues/105>
+
+This seems certianly wrong; it's lost the NOT.
+
+	  addAssociatedFile ik f = queueDb $ do
+	  	-- If the same file was associated with a different key before,
+	  	-- remove that.
+	-	delete $ from $ \r -> do
+	-		where_ (r ^. AssociatedFile ==. val af &&. not_ (r ^. AssociatedKey ==. val ik))
+	+	deleteWhere [AssociatedFile ==. af, AssociatedKey ==. ik]
+
+Otherwise, looking over the patch, it looks like it probably does the same thing,
+but my memory of persistent's SQL DSL is fuzzy and I'm not 100% sure.
+Is there any way to verify the same or equivilant SQL gets generated?
+
+One hunk I'm unsure about:
+
+	- removeExportTree h k loc = queueDb h $ 
+	- 	delete $ from $ \r ->
+	- 		where_ (r ^. ExportTreeKey ==. val ik &&. r ^. ExportTreeFile ==. val ef)
+	+ removeExportTree h k loc = queueDb h $
+	+ 	deleteWhere [ExportTreeKey ==. ik, ExportTreeFile ==. ef]
+
+Not clear from persistent's documentation if deleteWhere with two
+Filters combines them with AND or OR? Looks like there's a FilterAnd
+that would clearly do the same thing as the esquelito version.
+
+Similarly unsure about the removeExportedLocation and removeAssociatedFile
+and addAssociatedFile hunks.
+
+Do we know that an empty Filter list matches everything as used below
+instead of nothing? Not clear to me from the documentation.
+
+	  dropAllAssociatedFiles :: WriteHandle -> IO ()
+	  dropAllAssociatedFiles = queueDb $
+	-  	delete $ from $ \(_r :: SqlExpr (Entity Associated)) -> return ()
+	+ 	deleteWhere ([] :: [Filter Associated])
+
+Of course I can test all of these questions, and it would be good
+to expand the test suite to test the SQL code in isolation, but
+maybe you know the answers already.
+"""]]