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 several typos #3724

Closed
wants to merge 1 commit into from
Closed

Fix several typos #3724

wants to merge 1 commit into from

Conversation

@rockdaboot
Copy link
Contributor

@rockdaboot rockdaboot commented Apr 3, 2019

Please let me know if you prefer a more detailed commit message.

The spell-checking script follows tomorrow...

@jzakrzewski
Copy link
Contributor

@jzakrzewski jzakrzewski commented Apr 3, 2019

Hmmm... also AFAIK the impacket is just copied Python module and as such should probably be left alone.

@rockdaboot rockdaboot force-pushed the rockdaboot:tmp-fix-typos branch from 1b63c11 to a0c3c88 Apr 3, 2019
@rockdaboot
Copy link
Contributor Author

@rockdaboot rockdaboot commented Apr 3, 2019

@jzakrzewski Thanks for review. Could you point out which files / directories in the repo are copied (=will be overwritten eventually) and where they come from ? I would put them as exceptions into the spellcheck script, also it might be a good idea to report the typos upstream.

@jzakrzewski
Copy link
Contributor

@jzakrzewski jzakrzewski commented Apr 3, 2019

Well, at least everything in the tests/python_dependencies/impacket. I'm not aware of anything else at the moment.
Maybe you could contribute your fixes to the impacket itself?

@rockdaboot
Copy link
Contributor Author

@rockdaboot rockdaboot commented Apr 3, 2019

Sure, let's see what tomorrow brings...

@@ -1586,7 +1586,7 @@ static CURLcode myssh_statemach_act(struct connectdata *conn, bool *block)
return CURLE_BAD_DOWNLOAD_RESUME;
}
}
/* Does a completed file need to be seeked and started or closed ? */

This comment has been minimized.

@danielgustafsson

danielgustafsson Apr 3, 2019
Member

I know seeked isn't the correct spelling, but in this case I'm ready to give it the benefit of being wrong since it's referring to the specific seek call.. The current spelling, in all it's typoness, convey meaning.

This comment has been minimized.

@rockdaboot

rockdaboot Apr 3, 2019
Author Contributor

Reasonable, I'll revert it tomorrow.

@@ -2232,7 +2232,7 @@ static CURLcode ssh_statemach_act(struct connectdata *conn, bool *block)
return CURLE_BAD_DOWNLOAD_RESUME;
}
}
/* Does a completed file need to be seeked and started or closed ? */
/* Does a completed file need to be sought and started or closed ? */

This comment has been minimized.

@danielgustafsson

danielgustafsson Apr 3, 2019
Member

Same as above.

This comment has been minimized.

@rockdaboot

rockdaboot Apr 3, 2019
Author Contributor

Same as above ;-)

@@ -815,7 +815,7 @@ def recv_packet(self, timeout = None):
# The next loop is a workaround for a bigger problem:
# When data reaches higher layers, the lower headers are lost,
# and with them, for example, the source IP. Hence, SMB users
# can't know where packets are comming from... we need a better

This comment has been minimized.

@danielgustafsson

danielgustafsson Apr 3, 2019
Member

impacket is vendored code and should remain untouched, please submit these to the upstream project.

This comment has been minimized.

@rockdaboot

rockdaboot Apr 3, 2019
Author Contributor

See conversation above

@danielgustafsson
Copy link
Member

@danielgustafsson danielgustafsson commented Apr 3, 2019

No need to spend the effort of rebasing this for such small details, I fixed up the commit for you and pushed. Thanks for your contribution!

@rockdaboot rockdaboot deleted the rockdaboot:tmp-fix-typos branch Apr 4, 2019
@rockdaboot
Copy link
Contributor Author

@rockdaboot rockdaboot commented Apr 4, 2019

Nice, thank you !

@lock lock bot locked as resolved and limited conversation to collaborators Jul 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.