Skip to content

Conversation

@JoeZiminski
Copy link
Member

No description provided.

Copy link
Member

@adamltyson adamltyson left a comment

Choose a reason for hiding this comment

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

I haven't gone through in too much detail, but all looks good. I've left a few specific comments. More generally, it would be useful to start adding docs and tests now so it's easier for other to test it as it develops.

manager.py is also a very long file, and hard to review. I'd move as much code out into other modules as possible.

Lastly, the code line length is formatted, but it would be good to do the same with the docstrings. Some are very long.

@JoeZiminski
Copy link
Member Author

manager.py is also a very long file, and hard to review. I'd move as much code out into other modules as possible.

agree, have factored out configs and some utils. ATM utils of different type (ssh, directory managerment) are combined into a utils file but will be worth factoring out of separate files as it gros.

Lastly, the code line length is formatted, but it would be good to do the same with the docstrings. Some are very long.

thanks have done.

Am currently writing tests which will be useful to see how optimal pyftpsync is for our needs. It might be worth using rysnc / robocopy directly rather than pyftpsync which uses sftp commands directly, but doesn't provide as much extra checking (e.g. checksum options).

@adamltyson
Copy link
Member

Am currently writing tests which will be useful to see how optimal pyftpsync is for our needs. It might be worth using rysnc / robocopy directly rather than pyftpsync which uses sftp commands directly, but doesn't provide as much extra checking (e.g. checksum options).

Sounds good. I guess there's a package out there for doing these checks?

@JoeZiminski JoeZiminski merged commit 3c77c94 into main Sep 30, 2022
@JoeZiminski JoeZiminski deleted the add_project_folders_framework branch October 7, 2022 16:35
@JoeZiminski JoeZiminski restored the add_project_folders_framework branch October 18, 2022 13:54
@JoeZiminski JoeZiminski deleted the add_project_folders_framework branch October 18, 2022 13:56
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.

3 participants