SFTP and More#78
Conversation
|
Awesome!!!! And make sure to also test on Linux under mono. |
|
Done! If you want, please add me to contributors so I can do some more refactoring / add more features / solve issues 😄 |
| { | ||
| SFTP sftp = new SFTP(host.Text, user.Text, password.Text, int.Parse(port.Text)); | ||
| sftp.UploadSFTPFile( | ||
| Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetEntryAssembly().Location), |
There was a problem hiding this comment.
Instead of this, just generate a test file with some random content in it, we don't want to ship any more data to the client than needed. Use guids to avoid file name conflicts
| UploadToFTPServer.Checked = false; | ||
| return; | ||
| } | ||
| }*/ |
There was a problem hiding this comment.
All of these comments shouldn't need to be in the source anymore
There was a problem hiding this comment.
Thought It would be better to let it there, so if you wanted do display a message like you did with ftp. What do you think about it?
There was a problem hiding this comment.
Either show the message or remove the code, leaving the code there would just cause confusion.
| @@ -0,0 +1 @@ | |||
| Upload OK No newline at end of file | |||
There was a problem hiding this comment.
Again, really don't like that we have to ship this file around.
There was a problem hiding this comment.
Please see the added comments, I especially don't like the idea of shipping around a txt file with the install to test upload, much better to just generate it, and delete it as needed. And use Guid.NewGuid() to get a random string to the name, to avoid conflicts with other files named test
|
Also, did you test it on linux? |
|
I didn't test on linux but, since we're using SSH.NET, it seems thet everything will work |
|
Done |
zlepper
left a comment
zlepper
left a comment
There was a problem hiding this comment.
Still a couple of things that needs to be correctly, but they are mostly small and stylistic
| this.label6.Name = "label6"; | ||
| this.label6.Size = new System.Drawing.Size(205, 13); | ||
| this.label6.TabIndex = 13; | ||
| this.label6.Text = "--> Make sure there is no file named \"test\""; |
| System.IO.StreamWriter file = new System.IO.StreamWriter(filePath); | ||
| file.WriteLine(lines); | ||
|
|
||
| file.Close(); |
There was a problem hiding this comment.
Files should really be opening using the "using" pattern https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/using-statement, as this will ensure automatic closing of the file. As i haven't done it everywhere in the project, then i will not stop the pull request just from this.
I'm writing this comments mostly to make sure you learn it for future reference :)
There was a problem hiding this comment.
Ok, thanks. I knew about it, but I don't like using statements, I prefer .Dispose();. Will do it now too.
There was a problem hiding this comment.
The reason for using the using statement is that if something goes horribly wrong and crashes before you call dispose, then the file is still closed, and not left hanging around.
|
|
||
| // Write the string to a file. | ||
| Guid g = Guid.NewGuid(); | ||
| string uuid = Convert.ToBase64String(g.ToByteArray()); |
There was a problem hiding this comment.
Instead of this, just to string guid = Guid.NewGuid().ToString() it will have the same result, and you won't get the extra base64 chars
|
Looking good so far. I will still need to confirm manually that everything works on linux, but that will not be today. |
|
Done (Again) |
|
Looks good to me now. During the weekend I will get it merged, provided it works on Linux. |
|
Bump for linux testing of this PR |
4 participants
Hey Guys!
*It's faster only sometimes
This PR:
TESTED