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/exclude add when initial zappa setting #1242

Merged
merged 11 commits into from
Nov 10, 2023

Conversation

kyaryunha
Copy link
Contributor

@kyaryunha kyaryunha commented May 4, 2023

Description

My prev pull request is bad because it always disable boto.

So I code a new idea about this mention.
#1235 (comment)

GitHub Issues

My Prev Pull Request: #1235
Issue: #1241

Copy link
Collaborator

@monkut monkut left a comment

Choose a reason for hiding this comment

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

Can you add a testcase to confirm the results of this?

@coveralls
Copy link

coveralls commented May 8, 2023

Coverage Status

coverage: 74.736%. remained the same
when pulling 3747f83 on kyaryunha:zappa-settings-add-exclude
into dd5d1f3 on zappa:master.

@hellno hellno self-requested a review May 8, 2023 08:24
@kyaryunha
Copy link
Contributor Author

I wrote a test code to check if excludes are properly included in zappa_settings when I run zappa init.

It's the first time I've written test code in Python, so I'd appreciate it if you could check if the test code was written clearly.

In addition, when there is something in the exclude, please tell me if you need to write a test code to see if it is excluded well in the zip.

thank you!

@sridhar562345
Copy link
Contributor

@kyaryunha
This pull request is not backward compatible and might affect users who have older zappa_settings.json.

I feel it should be labeled as a breaking change.

@kyaryunha
Copy link
Contributor Author

Yes, this code conflicts with the old zappa_settings.json.
I think it would be nice to do it when updating a large version.

@kyaryunha
Copy link
Contributor Author

Ah.. ci failed...
I'll think about it again and fix it this week.

@kyaryunha
Copy link
Contributor Author

I fixed my test code. Could you please run ci again?

zappa/cli.py Show resolved Hide resolved
@kyaryunha kyaryunha mentioned this pull request Sep 11, 2023
@monkut monkut merged commit 581144c into zappa:master Nov 10, 2023
6 checks passed
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.

5 participants