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

Fix often forget zip python package #113

Merged

Conversation

YayunHuang
Copy link
Contributor

Ref: MUP-164

Makefile Outdated
@@ -71,6 +72,13 @@ check-format:
bundle-python:
Copy link
Collaborator

@pkong-ds pkong-ds Aug 26, 2024

Choose a reason for hiding this comment

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

can you help change bundle-python as bundle-python-mac too? thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pkong-ds updated

Makefile Outdated
Comment on lines 77 to 86
mkdir -p mockup
cp -r mockup_package/mockup/* mockup/
zip -r public/mockup.zip mockup
rm -rf mockup

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, forgot to say - if we are creating&removing sth in a script, can we follow the convention of putting these into tmp/ folder? thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pkong-ds updated~

@YayunHuang YayunHuang force-pushed the 164-fix-often-forgoot-zip-python-package branch from be970cd to df9dc33 Compare August 26, 2024 08:22
@YayunHuang YayunHuang force-pushed the 164-fix-often-forgoot-zip-python-package branch from df9dc33 to 618b639 Compare August 26, 2024 08:39
Makefile Outdated
Comment on lines 79 to 86
cd tmp && \
if [ -d "mockup" ]; then \
zip -r ../public/mockup mockup; \
else \
echo "Error: Directory 'mockup' does not exist."; \
exit 1; \
fi

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why need cd into tmp?

Suggested change
cd tmp && \
if [ -d "mockup" ]; then \
zip -r ../public/mockup mockup; \
else \
echo "Error: Directory 'mockup' does not exist."; \
exit 1; \
fi
if [ -d "tmp/mockup" ]; then \
zip -r public/mockup tmp/mockup; \
else \
echo "Error: Directory 'mockup' does not exist in directory 'tmp'."; \
exit 1; \
fi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pkong-ds if we use zip -r public/mockup tmp/mockup; the unzip file will be

tmp/
    mockup/

Copy link
Collaborator

Choose a reason for hiding this comment

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

okay, let's keep it then

Copy link
Collaborator

@pkong-ds pkong-ds left a comment

Choose a reason for hiding this comment

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

Added rm -rf tmp, @YayunHuang please merge if okay!

@YayunHuang YayunHuang merged commit 7292e5c into oursky:main Aug 26, 2024
1 check passed
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.

2 participants