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

Add 'preserve' argument #20

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

Conversation

chientrm
Copy link

@chientrm chientrm commented Feb 7, 2022

New argument:

  • Name: preserve
  • Type: boolean
  • Default: false
  • Example: true
  • Description: Preserve existing files and folders on server instead of deleting them

@SamKirkland
Copy link
Owner

Thanks for the PR Chien!

This is a popular request so thanks for putting together a PR.

I do believe having 3 arguments for this would be the most flexible approach. Here is my rational.
This script has 3 main cycles. Uploading new files. Replacing files that have been updated/changed. And deleting old files.

Your specific usecase requires opting-out of the delete step... But what about the replace? Are you okay with that running? Will other use cases?

So I'm thinking 3 props. Basically allowing people to opt out of any given step to fit their specific usecase.
delete
upload
replace

We could default these to true. Let's try to get the naming right... delete does not seem descriptive enough. Maybe allowDelete? What are your thoughts?

Also we should keep this logic with the hash diff implementation (I think). I'm on mobile right now so it's hard to say without looking at the codebase.

@chientrm
Copy link
Author

chientrm commented Feb 7, 2022

We could default these to true. Let's try to get the naming right... delete does not seem descriptive enough. Maybe allowDelete? What are your thoughts?

Currently, the argument --exclude is working very well.

I guess --exclude-upload, --exclude-replace, and --exclude-delete would be what we're looking for.

The default value for them would be excludeDefaults which is same with --exclude

In this case

  • --exclude-upload **/*.* will be equivalent to allowUpload: false
  • --exclude-replace **/*.* will be equivalent to allowReplace: false
  • --exclude-delete **/*.* will be equivalent to allowDelete: false

I don't know how glob pattern works exactly but that's the best I can think of.

@SamKirkland
Copy link
Owner

exclude is weird because now it's a double negative. Also those glob patterns are complex to understand so boolean is preferred. I'm thinking allowXXX is the best

As for implementation we can skip processing files based on the users settings in this file.
I prefer skipping in getDiffs over mutating the results.

if (fileNameCompare < 0) {

@chientrm
Copy link
Author

chientrm commented Feb 7, 2022

exclude is weird because now it's a double negative. Also those glob patterns are complex to understand so boolean is preferred. I'm thinking allowXXX is the best

Yup exclude is kinda overcomplicated. My current use cases only require basic true/false activation.

As for implementation we can skip processing files based on the users settings in this file.
I prefer skipping in getDiffs over mutating the results.

Working on it!

@chientrm
Copy link
Author

chientrm commented Feb 9, 2022

So you will update on FTP-Deploy-Action, too?

@chientrm
Copy link
Author

chientrm commented Feb 9, 2022

Hi @SamKirkland , Please checkout 085feeb

@rpatni5
Copy link

rpatni5 commented Nov 9, 2023

When this option will be available, it' still open ?

@miabid
Copy link

miabid commented Aug 7, 2024

Any updates on this really useful feature? Seems it's still not merged.

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.

4 participants