-
Notifications
You must be signed in to change notification settings - Fork 364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Typos #852
Fix Typos #852
Conversation
rephrase warning message in README.md
@kwwall I hope this is now manageable. Only 54 commits! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM; just a few places where I requested some minor changes.
@@ -198,7 +198,7 @@ private Integer parseNumber( PushbackSequence<Integer> input ) { | |||
sb.appendCodePoint( c ); | |||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good now.
Fix Issue #851