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

Confusing and missing API docs? #1

Closed
sbrl opened this issue Jan 14, 2025 · 9 comments
Closed

Confusing and missing API docs? #1

sbrl opened this issue Jan 14, 2025 · 9 comments
Assignees
Labels
documentation Improvements or additions to documentation question Further information is requested

Comments

@sbrl
Copy link

sbrl commented Jan 14, 2025

Hello,

I'm currently using this package in a project, but it is proving to be a difficult task as the docs available are rather limited.

The README does not contain any obvious links to documentation, and while I can search the codebase on how to use e.g. sftp.write(), all I can find is https://github.com/NobleMajo/hivessh/blob/main/src/essentials/SftpPromiseWrapper.ts#L136 and https://github.com/NobleMajo/hivessh/blob/main/src/essentials/SftpPromiseWrapper.ts#L400.

In the README, I see the following: https://github.com/NobleMajo/hivessh/#technologies

.....but it is not immediately obvious that the API for hivessh is the same or different to that of the ssh2 package.

The ssh2 package docs suggest that all SFTP methods are uppercase: https://github.com/mscdex/ssh2/blob/master/SFTP.md

....but that is not the case in hivessh (SSHHost.sftp), and there is not a link to the above, nor an explanation of the differences between hivessh and ssh2's implementation thereof. If method names have changed, have e.g. arguments etc changed too? The only way to tell is to go digging around in hivessh's code, which is not a helpful or time-efficient solution.

To this end, I suggest having a section that:

a) Links to the relevant docs, even if they are from another package
b) Explains the relationship between the other packages and hivessh
c) Outlines and details the differences between hivessh and the other package.

It would be nice to have some docs on how to use sftp if possible as it is not obvious from the above how to use it (especially for e.g. writing to disk), but I understand this would be more effort than the above.

Copy link

👋 Hello!

Welcome to the repository, and thank you for opening an issue. 🎉 We're excited to have you contribute! Please make sure to include all the relevant details to help us understand your report or suggestion.

If you’re new here, take a moment to review our contribution guidelines and code of conduct. These documents will help you collaborate effectively and ensure a positive experience for everyone.

We're here to help—feel free to reach out if you have any questions. 🚀

Thank you, and happy coding! 💻

@sbrl
Copy link
Author

sbrl commented Jan 14, 2025

Hey, @github-actions! Thanks for the links. However, they do not appear to work for me, as they link to a search of the issues list.

/cc @NobleMajo as the owner of this repo: I think the bot is broken.

Also, what do you mean by feel free to reach out if you have any questions? Is a GitHub issue not considered reaching out, or is there another preferred method to use here? From this I am unclear on the protocol you expect people to follow.

@NobleMajo
Copy link
Owner

Hii,
i will check the issue later.

But i think its already a good idea to create a ssh2 and hivessh comparison.

THX sbrl

@NobleMajo NobleMajo self-assigned this Jan 14, 2025
@NobleMajo NobleMajo added documentation Improvements or additions to documentation question Further information is requested labels Jan 14, 2025
@NobleMajo
Copy link
Owner

NobleMajo commented Jan 15, 2025

Hii @sbrl,
as for the documentation, i hope my recent changes have cleared up some misunderstandings and made it more obvious that the core functions are a wrap around of the ssh2-library.
https://github.com/NobleMajo/hivessh
If you sill find some potential risks for misunderstanding, please let me know where and why.

As for the sftp functions, i wrapped the ssh2-library with a promise-based approche.
Shy the ssh2-library documents a few functions in uppercase I don't quite understand either, but in javascript (or generally in the c-influenced code world) you can assume camel case for function names (if a language doesn't define anything specific, like golang).
I have used the typescript types and interfaces of the ssh2-library, which explicitly and understandably use lowercase and i have adapted them for promises, so yes the callback parameters and return values have changed.

I hope that the following types will help you: https://www.npmjs.com/package/hivessh/file/fd0e59bb46b481ba4758921a292de6d5ef3437ec849a896f673ab91f6f8059da
(Or here at /dist/essentials/SftpPromiseWrapper.d.ts)

I'll check where i can add the link above in the readme together with: https://github.com/mscdex/ssh2/blob/master/SFTP.md

The actions bot greet message is now changed, thanks for pointing out the problems and misunderstandings with the bot.
In the future I'll also add some information about the protocol I expect people to follow, but I think in the end I'll just tell people to relax and wait for a response.

I appreciate your effort and look forward to collaborating with you! 🚀
Cheers ❤️

@sbrl
Copy link
Author

sbrl commented Jan 22, 2025

Hello,

Thanks for updating in 6b14901.

That does help to make it clearer.

It isn't really very helpful if you are looking through the examples though, and doesn't link to the documentation for the ssh2 library, which leaves people at a dead-end when reading your documentation if they have an issue.

For example:

  1. I read the hivessh README, and look at the Sftp header: https://github.com/NobleMajo/hivessh/?tab=readme-ov-file#sftp
  2. I have an additional question that isn't covered by the example shown
  3. I know that hivessh library wraps the ssh2 library, but I don't have a link to that documentation.
  4. I somehow go goolging around and searching NPM to find it, and then gegt confused because of the uppercase usage in their docs
  5. I end up looking at hivessh's code to see what the problem is

you can assume camel case for function names

Indeed, that makes sense...... but your README / docs don't mention this :P

Another example:

  1. I read the hivessh README, and look at the .exec() examples.
  2. I get confused about what properties are available on the return object
  3. I then have to manually locate the ssh2 documentation as above, potentially cross-referencing it with hivessh's code. As a user I may not even find the right documentation, causing further confusion

To this end, it is suggested that the existing examples contain a short sentence explaining where further documentation can be found - including a link?

(also, the table of contents in the README needs updating :-))

Sorry to be a pain, @NobleMajo!

@NobleMajo
Copy link
Owner

Hey @sbrl ,
your not a pain to me.
Kind contributions and recomendations are always welcome.

I check you recomendations tomorrow.

THX, cya

@NobleMajo NobleMajo reopened this Jan 22, 2025
@NobleMajo
Copy link
Owner

Btw, your welcome to create a PR If u wanna :)

@NobleMajo
Copy link
Owner

Hii @sbrl ,
i have added 2 links to the readme (a ssh2 sftp doc and a hivessh types link).
1a0bdde

Because "you can assume camel case for function names": i mean you can assume that without any library documentation. otherwise i (or any other npm or js library) would soon have to put a best practice coding and at best a how to write js guide in the readme/documentation. that is out of scope.

Toc tree comes :)

Your welcome to create PR's :)
Thanks and cya

@sbrl
Copy link
Author

sbrl commented Jan 30, 2025

Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants