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 #3166

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Fix typos #3166

merged 1 commit into from
Apr 10, 2024

Conversation

kianmeng
Copy link
Contributor

@kianmeng kianmeng commented Apr 4, 2024

What problem is this PR intended to solve?
Fix typos found via codespell -S test,tmp,ports,coverage,googletest -L te,reencode,ocur and typos --format brief

Have you included adequate test coverage?
Existing tests are sufficient.

Does this change affect the behavior of either the C or the Java implementations?
Nope.

Copy link
Contributor

@stevecheckoway stevecheckoway left a comment

Choose a reason for hiding this comment

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

Most of this looks great! I don't understand the change to the test file though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this file have been changed? It's not obvious to me what is being changed here.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, this file should not be changed, it's an artifact used by tests.

test/helper.rb Outdated
@@ -154,7 +154,7 @@ def initialize_nokogiri_test_gc_level
GC.compact
rescue NotImplementedError
@@gc_level = "normal"
warn("#{__FILE__}:#{__LINE__}: GC compaction not suppport by platform")
warn("#{__FILE__}:#{__LINE__}: GC compaction not support by platform")
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this should be "supported"

Copy link
Member

Choose a reason for hiding this comment

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

Agree.

public static class NokogiriXInlcudeEntityResolver implements org.xml.sax.EntityResolver {
public static class NokogiriXIncludeEntityResolver implements org.xml.sax.EntityResolver {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what, if any, impact changing the name of this public class has so I just thought I'd note it.

Copy link
Member

Choose a reason for hiding this comment

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

This class is commented out since f73bd4e (2017-02-15) so no worries about this change.

Copy link
Member

@flavorjones flavorjones left a comment

Choose a reason for hiding this comment

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

Please remove the changes to the test artifact.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, this file should not be changed, it's an artifact used by tests.

test/helper.rb Outdated
@@ -154,7 +154,7 @@ def initialize_nokogiri_test_gc_level
GC.compact
rescue NotImplementedError
@@gc_level = "normal"
warn("#{__FILE__}:#{__LINE__}: GC compaction not suppport by platform")
warn("#{__FILE__}:#{__LINE__}: GC compaction not support by platform")
Copy link
Member

Choose a reason for hiding this comment

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

Agree.

Found via `codespell -S test,tmp,ports,coverage,googletest -L
te,reencode,ocur` and `typos --format brief`
Copy link
Member

@flavorjones flavorjones left a comment

Choose a reason for hiding this comment

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

Looks good, thank you again!

@flavorjones flavorjones merged commit 8222c46 into sparklemotion:main Apr 10, 2024
126 checks passed
@kianmeng
Copy link
Contributor Author

🥳 🥳 🥳 🥳 🥳

@myabc myabc mentioned this pull request May 16, 2024
flavorjones added a commit that referenced this pull request May 16, 2024
**What problem is this PR intended to solve?**

Fix various typos - some found via `codespell` and `typos` CLI (h/t
@kianmeng with PR #3166)

**Have you included adequate test coverage?**

This PR does not change test coverage - existing tests should suffice.

**Does this change affect the behavior of either the C or the Java
implementations?**

This PR does not introduce any changes in behavior.
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.

None yet

3 participants