Conversation
rephrase warning message in README.md
|
@kwwall I hope this is now manageable. Only 54 commits! |
kwwall
left a comment
kwwall
left a comment
There was a problem hiding this comment.
LGTM; just a few places where I requested some minor changes.
| input.next(); | ||
|
|
||
| // if character is a semi-colon, eat it and quit | ||
| // if character is a semicolon, eat it and quit |
There was a problem hiding this comment.
Okay, I'll let this pass, but this sort of comment is not "public" in the sense that it's not going to end up in generated Javadoc. One of my reasons wanting to separate this type of comment from Javadoc comments is that we don't need to scrutinize these as much.
There was a problem hiding this comment.
Oh sh*t, I have cherry picked my way through all the commits and went through them again a second time, dropping all of them that did not match your criteria. This one commit must have slipped through. That was not my intention, and if you want, I can revert this commit again.
There was a problem hiding this comment.
It's fine. There's only a few of those here and I reviewed them all. In your original PR there were a lot of these types, and I'm just saying as a rule, I really don't care that much if those "internal comments" are ever fixed unless they are flat out misleading (e.g., like they left out a 'not') so the logic of the comment is reversed or no one can really understand what the comment meant. But only people working on ESAPI code itself generally pay attention to that code. So, really low priority to fix these.
2 participants
Fix Issue #851