Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Modernize the rsync project #72

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jhundley9109
Copy link

This a fairly large rewrite of the project, mostly done by boneskull (https://github.com/boneskull/rsync) to move Rsync from an object to a more modern class based layout. However, I found the changes over there addressed a few of the open issues on this project and I fixed up a few other small issues and bugs with boneskull's changes. Additionally, I've updated all the npm dependencies to the latest versions and ran the semistandard code standards across the project.

Open issues addressed in these changes include:
#69 #66 #61 #58 (Though this one was already solved and actually is set via the executable() function or as a parameter to new Rsync()) and #47

The API has changed in two major ways with these changes:
1: flags() has now become .setFlags() and .unsetFlags().
2: the .execute() function parameters and return have changed. It always returns a Promise now however, there is no way to return the original spawn process to the caller now. I'm not 100% sure it is necessary to have the ability to pass it up, but if it is that could be worked around as boneskull had it before I was in there mucking about. If this is necessary functionality, I'd be curious to look into how other projects have solved this without a kludgy use of callbacks. Perhaps that's the only real way to solve it...

My main objective with this pull request is to get feedback on the changes. If this project is abandoned perhaps I should submit a new npm rsync2 module or better yet I can just be a contributor on this project if any maintainers/admins are still active and create a version 2 release with these changes. Please let me know your thoughts.

I am using my forked version of boneskull's fork for my jsyncd project over here (https://github.com/jhundley9109/jsyncd).

boneskull and others added 7 commits November 13, 2017 00:32
Signed-off-by: Christopher Hiller <[email protected]>
…he callback to rsync.execute() to always return a promise. This simplifies the api a bit. Fixed a bug where double quotes were causing an bug where the rsync process wouldn't run. Fixed a test with checking the executable path. Ran the semistandard filter on the project. Had to slightly modify the module.exports to play nice with import { Rsync } from 'rsync'
…e there have been two sets of api changing updates...
…rsync parameters as functions) and setting of flags is backwards compatible by calling rsync.flags() or passing 'flags' to Rsync.build() but not unsetting of flags. You would still have to call unsetFlag() and simply passing 'false' as the last parameter wouldn't unset the flag.
@jhundley9109
Copy link
Author

jhundley9109 commented Sep 28, 2021

I have improved the .set() function to also accept an array of arrays for long options. As it stands now, each long option has to be set via
rsync.set('no-perms')
rsync.set('no-group')
rsync.set('chmod', 'Dg+s')

This does not work with the Rsync.build() command (which just so happens to be the way I want to use the library)

So now you can do something like this
rsync.set([['no-perms'], ['no-group'], ['chmod', 'Dg+s'])
or
Rsync.build(
set: [['no-perms'], ['no-group'], ['chmod', 'Dg+s']],
etc...
)

This change would be backwards compatible.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants