David Haines

  • More
  • CR-31
  • finished reviewing
This does get complicated. I wouldn't have thought of this case: "Colleen Flanagan <flanagan@umich.edu>" I'm suspect we don't need to deal with this now. I suggest adding this situation (an odd FR...

This does get complicated. I wouldn't have thought of this case: "Colleen Flanagan &lt;flanagan@umich.edu>"

I'm suspect we don't need to deal with this now. I suggest adding this situation (an odd FROM value) to the cases that the BSA should check.
Also the BSAs should be clear that an email with a missing date will have the import time and date used instead. This would change the location of the
message in the archive.

This is fine if there are no headers, but what if there are headers but one or more of the critical ones are missing? Log some information (archive name?) when have to add headers so we know how b...

This is fine if there are no headers, but what if there are headers but one or more of the critical ones are missing?

Log some information (archive name?) when have to add headers so we know how big a problem this is. Or don't log it. But if the problem occurs again we should start logging when special processing is required.

Good. Allocating each time avoids the problem of not being thread safe.

Good. Allocating each time avoids the problem of not being thread safe.

  • More
  • CR-29
  • finished reviewing
Add a space before 'attachment'

Add a space before 'attachment'

Is this overwriting an existing value for item.url?

Is this overwriting an existing value for item.url?

The code looks good. However I wonder why returning the content rather than the url was every the right thing to do.

The code looks good. However I wonder why returning the content rather than the url was every the right thing to do.

Also include a version of the URLs that would work for the command line server version. I suspect that people will try that first.

Also include a version of the URLs that would work for the command line server version. I suspect that people will try that first.

Some place needs to explain what the purpose / contents of the test files.

Some place needs to explain what the purpose / contents of the test files.

An quick overview section on how data flows through the stub server would be helpful. E.g. list what query types it responds to, what data matters, how it looks for the data to return, what needs t...

An quick overview section on how data flows through the stub server would be helpful. E.g. list what query types it responds to, what data matters, how it looks for the data to return, what needs to be in the data files.

Add "FROM THE COMMAND LINE" to the title

Add "FROM THE COMMAND LINE" to the title

Clarify that at this point Qualtrics-LTI is pointing to the real Qualtrics.

Clarify that at this point Qualtrics-LTI is pointing to the real Qualtrics.

spelling

spelling

Clarify if need to change web.xml in source, or in the expanded webapp, or if there is some other way.

Clarify if need to change web.xml in source, or in the expanded webapp, or if there is some other way.

This comment is separate from the rest of the review. You may want someone to look over the Ruby install page for clarity / generality.

This comment is separate from the rest of the review. You may want someone to look over the Ruby install page for clarity / generality.

QUALTRICS-255 moved from trunk to branch
QUALTRICS-255 moved from trunk to branch
Indentation is off in a few places.

Indentation is off in a few places.

Returning after it is found that there is no person would be clearer (if correct).

Returning after it is found that there is no person would be clearer (if correct).

Should be "There is no news event" or "There are not news events".

Should be "There is no news event" or "There are not news events".

When will the sakaiUserId be null?

When will the sakaiUserId be null?

Indentation is off.

Indentation is off.

Ship it.

Ship it.