On Jul 5, 10:32 pm, Mackram <[email protected]> wrote:

> In the mean time any things that do not look good please let me know.

Please try to put only things that belong together into your commits.
'hg record' is helpful for doing this.

Likewise please provide specific commit messages that show what
has changed in a synoptical way.

These two things make it easier for people to review your commits
and identify problems later.


Comments on the code follow. I've reviewed them starting with
the oldest so it might happen that I criticize things that
you have fixed or changed in later commits.

* please try to keep your lines (including docstrings) roughly to 80
  characters.

* define-client-backend: not sure if this should be a DEFVAR instead
  of a DEFPARAMETER...

* make-backend-dependency: I haven't checked more closely but
  it seems to contain a lot of code already found in make-local-
dependency
  (or some other dep function). Is there a way to merge them?

* build-client-backend: let's use NCONC here instead of APPEND.
  This dep stuff is in the performance-critical path.

* build-client-backend: use WHEN if your IF only has one consequent.
  This also leads to the more idiomatic
  (loop for file in (files backend)
        when (string/= ...)
        collect file)

* webapp class, slot javascript-backend: provide an INITFORM here.
  We can't run without a backend. JQuery is a good default.

  This also removes the conditional

    (if (weblocks-webapp-javascript-backend app)

  below.

* test HCR-10 (I think): prototype isn't quoted. This test
  will fail.

* 6ebd733f23b9: do we need the *.0.* files in the repository?
  (This is not a rhetorical question, I'm unsure myself)

* fb8f5271c109: you've changed the webapp description of the
  demo into a meaningless string. Please revert to the old value
  "A web application based on Weblocks" or a better one based on
  that.

* ibid.: alert("Oops. You do not have a backend defined")
    -- let's abstract that into a function called whenever
    there's no backend. We should also take into account
    partial implementations -- some backends might not
    have effects stuff for example.

* ibid.: please trim down the number of chars per line here
    as well.

* ibid. (src/dependencies.lisp): NCONC as well.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"weblocks" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/weblocks?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to