a few comments from tr pr descriptions

chapter_15_simple_form.asciidoc

@@ -18,6 +18,14 @@ and making sure each of them tests only one thing at a time.
 // - that we're indeed gonna do be doing some refactor 
 // and in such a case functional tests shield us.
 
+// DAVID: Keeping the test suite passing
+
+// One of the best things I've taken from our recent TDD dojos is the idea of
+// keeping the test suite passing during refactors, even to the point of doing
+// apparently counter-intuitive things such as copy-pasting code. In this chapter
+// we have the test suite failing for quite a while (even to the point where we're
+// making git commits while the suite is failing).
+
 === Moving Validation Logic into a Form
 
 TIP: In Django, a complex view is a code smell.

chapter_16_advanced_forms.asciidoc

@@ -24,6 +24,11 @@ Make sure you take a quick look at the
 and the <<what-to-test-in-views,recap on testing views>> at the end.
 // DAVID: should this be 'silliness'? That's the word you use in the aside.
 
+// DAVID: A general point: I feel like just because we're doing TDD
+// doesn't mean we can't occasionally start up the application
+// and use it to figure out what's happening.
+// It feels like a long time since we've done that!
+
 === Another FT for Duplicate Items
 
 
@@ -866,7 +871,7 @@ classes. Although it would have been nice to minimise hand-written HTML and
 use Django instead, it seems like we need to bring back our custom
 `<input>` and add a few attributes manually:
 
-// JAN: Can't you simply add the end version of the code below? These Git views are awful to read and even worse for copy&paste ... 
+// JAN: Can't you simply add the end version of the code below? These Git views are awful to read and even worse for copy&paste ...
 // CSANAD: I actually like these. They show quite clearly what to remove and what to add and IMO the reader also learns more from
 // typing these rather than just copying and pasting. If they really want to just copy and paste, then they could just open the
 // book-example repo anyway.
@@ -905,7 +910,7 @@ use Django instead, it seems like we need to bring back our custom
 ====
 
 <1> We hand-craft the `<input>` and the most important custom setting will be its
-    `class`. 
+    `class`.
 
 <2> As you can see, we can use conditionals even for providing additional `class` -es.footnote:[
     We've split the input tag across multiple lines so it fits nicely on the screen.

chapter_17_javascript.asciidoc

@@ -49,6 +49,14 @@ NOTE: I'm going to assume you know the basics of JavaScript syntax.
 // CSANAD: maybe we could also mention MDN
 // https://developer.mozilla.org/en-US/docs/Web/JavaScript#for_complete_beginners
 
+// DAVID: I have never before written a test in Javascript so I think this
+// chapter is really important, at least to give some flavour of what it's like.
+
+// DAVID: I think it would be improved with a high level overview of how
+// Jasmine works - I got it in the end,
+// but it would have been helpful at the beginning.
+
+
 
 === Starting with an FT
 

chapter_19_spiking_custom_auth.asciidoc

@@ -38,6 +38,38 @@ We'll use something fun called passwordless auth instead.
 Django's default auth module is ready and waiting for you.
 It's nice and straightforward, and I'll leave it to you to discover on your own.)
 
+////
+DAVID
+I like the way in this chapter you give a flavour of what it's like to give
+something a try without TDD - and then return to it.
+
+But - of all the chapters I've reviewed so far, I think this would most benefit
+with some more work. For me, reading the spike comes down to copy-pasting code
+that isn't really explained, and I don't think feel carried along with your
+thought process for that part of the chapter. So I think it would work better
+to move more of the explanation to the spike phase.
+
+It also might work better to conduct the spike a little differently. For
+example:
+
+You could start with laying out the high level design, with a sequence diagram.
+Then you could stub out the user interface (without, say, any actual email - it
+could just print the link out to the terminal). Get us to click around and
+actually run it. Then you could look at the User model. You could first look at
+the representation of the User and talk about why you don't want to use
+Django's built in. Then you could look at the Token, whether it could be a
+field on the User model versus separate, versus foreign key, versus not stored
+in the database at all and use a signed token instead. Finally you could figure
+out the actual sending of the email, maybe entirely separate by doing it from
+the Django shell.
+
+Once you've done all that design thinking, then it could be time to go back and
+TDD it...but it will make a lot more sense to the reader about the thought
+process you've been on. Use django.contrib.auth less?
+
+Side note: I wonder if it is worth experimenting with using django.contrib.auth even less for this use case, it feels like a slightly awkward fit and a bit difficult to understand what is going on. Then again, maybe not.
+////
+
 
 [role="pagebreak-before less_space"]
 === Passwordless Auth With "Magic Links"
@@ -379,6 +411,8 @@ on the staging server as well.
 
 ==== Storing Tokens in the Database
 
+// CSANAD (transcribed) you should probably hash the tokens
+
 ((("authentication", "storing tokens in databases")))
 ((("tokens")))
 How are we doing? Let's review where we're at in the process:
@@ -958,7 +992,9 @@ selenium.common.exceptions.NoSuchElementException: Message: Unable to locate
 element: input[name=email]; [...]
 [...]
 ----
+
 // JAN: I see the following exception: ModuleNotFoundError: No module named 'accounts', not the selenium one. We should remove all changes from settings.py/urls.py as well
+// HARRY - i think this is bc jan missed the 'git switch main' or earlier commit
 
 The first thing it wants us to do is add an email input element. Bootstrap has
 some built-in classes for navigation bars, so we'll use them, and include a

todos.txt

@@ -3,6 +3,10 @@
 
 # Later
 
+* hash the tokens
+
+
+
 ## switch to postgres
 
 - do it in deploy chaps