Skip to content
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

Merged
merged 57 commits into from
Sep 14, 2024
Merged

Fix Typos #852

merged 57 commits into from
Sep 14, 2024

Conversation

DarioViva42
Copy link
Contributor

Fix Issue #851

@DarioViva42
Copy link
Contributor Author

@kwwall I hope this is now manageable. Only 54 commits!

Copy link
Contributor

@kwwall kwwall left a 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.

src/main/java/org/owasp/esapi/Encryptor.java Outdated Show resolved Hide resolved
src/main/java/org/owasp/esapi/Encryptor.java Outdated Show resolved Hide resolved
src/main/java/org/owasp/esapi/StringUtilities.java Outdated Show resolved Hide resolved
src/main/java/org/owasp/esapi/User.java Outdated Show resolved Hide resolved
@@ -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
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

@kwwall kwwall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good now.

@kwwall kwwall merged commit cb02efe into ESAPI:develop Sep 14, 2024
1 check passed
@kwwall kwwall mentioned this pull request Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants