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

bash script for zipping plugin #25

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

chikkibum
Copy link

bash script for zipping wooCommerce plugin according to issue 6173

Copy link

@vaibhavjuspay vaibhavjuspay left a comment

Choose a reason for hiding this comment

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

@chikkibum I just reviewed the PR, added some comments please check those.

)


if [ -f "$ZIP_FILE" ]; then

Choose a reason for hiding this comment

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

Won't it be better to add a prompt asking the user whether to overwrite the existing file ?

plugin.sh Outdated

zip -r "$ZIP_FILE" "$PLUGIN_DIR" "${EXCLUDE_PARAMS[@]}"

echo "zip file created: $ZIP_FILE"

Choose a reason for hiding this comment

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

It will be better to check the exit status of the zip command as if there’s an issue with file permissions, disk space, or other problems, the script will continue and may report success even if the ZIP creation failed.

@chikkibum
Copy link
Author

I'll make the suggested changes the .

@sakksham7
Copy link

hey @chikkibum please remove the woocommerce-plugin.zip file.

@chikkibum
Copy link
Author

Will do

@gorakhnathy7
Copy link
Collaborator

Hey @chikkibum
Any update on the changes?

@chikkibum
Copy link
Author

Hey @chikkibum Any update on the changes?

Made the required changes as per the suggestions, if anything else can be made better do tell me .

Thank you

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

Successfully merging this pull request may close these issues.

4 participants