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

Root directory passed to NewServer is never used or acted on #65

Closed
sykesm opened this issue Dec 28, 2015 · 5 comments
Closed

Root directory passed to NewServer is never used or acted on #65

sykesm opened this issue Dec 28, 2015 · 5 comments

Comments

@sykesm
Copy link
Contributor

sykesm commented Dec 28, 2015

The NewServer constructor accepts and propagates a rootDir argument to the Server but the location is never used after construction. What was the intent of the argument? Could/should it be treated as the default working directory for the server instance?

@sykesm
Copy link
Contributor Author

sykesm commented Dec 28, 2015

Looks like #56 was related. I'd like to see the behavior described by @marksheahan implemented as we have linked the sftp server into an ssh daemon and would like to be able to set the default working directory of each instance to the home directory of the user. If this behavior is not implemented in the server, we will have to create a separate sftp server executable and exec it with the appropriate working directory.

If you're interested in a PR, I'd be happy to change the name of the argument to the constructor and make the necessary implementation changes.

Thanks.

@sykesm
Copy link
Contributor Author

sykesm commented Jan 14, 2016

Given the confusion over the meaning of the rootDir on the constructor and the fact that it's not implemented, perhaps it makes sense to remove it completely?

@davecheney
Copy link
Member

Yes, please remove it, then we can start from a fresh slate. Thanks

On Thu, 14 Jan 2016, 22:24 Matthew Sykes notifications@github.com wrote:

Given the confusion over the meaning of the rootDir on the constructor
and the fact that it's not implemented, perhaps it makes sense to remove it
completely?


Reply to this email directly or view it on GitHub
#65 (comment).

@mdlayher
Copy link
Member

Sounds good to me as well. I somehow didn't catch that it was totally unused when I was making my PRs. Apologies for that.

@sykesm
Copy link
Contributor Author

sykesm commented Jan 18, 2016

Addressed with 8e47c75.

@sykesm sykesm closed this as completed Jan 18, 2016
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

No branches or pull requests

3 participants