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

refactor: move settings to SettingsEnum #49

Merged
merged 19 commits into from
Jul 5, 2022
Merged

refactor: move settings to SettingsEnum #49

merged 19 commits into from
Jul 5, 2022

Conversation

TheJeterLP
Copy link
Contributor

See #37

@TheJeterLP TheJeterLP self-assigned this Jul 4, 2022
This was referenced Jul 4, 2022
@ghost
Copy link

ghost commented Jul 5, 2022

@TheJeterLP why you add "text_display_ad_with_themed_cta_banner_image"?

It should be already blocked by "ad_" generic reference (placed at the top of list).

It's checked by a contains istruction, should be good already. 🤷‍♂️

@TheJeterLP
Copy link
Contributor Author

@TheJeterLP why you add "text_display_ad_with_themed_cta_banner_image"?

It should be already blocked by "ad_" generic reference (placed at the top of list).

It's checked by a contains istruction, should be good already. 🤷‍♂️

Was just for debugging. Forgot to remove it from commit.

@ghost
Copy link

ghost commented Jul 5, 2022

@TheJeterLP why you add "text_display_ad_with_themed_cta_banner_image"?
It should be already blocked by "ad_" generic reference (placed at the top of list).
It's checked by a contains istruction, should be good already. 🤷‍♂️

Was just for debugging. Forgot to remove it from commit.

Anyway...have you understand how to block new elements? Was not much difficult at the end xD.

@TheJeterLP TheJeterLP marked this pull request as ready for review July 5, 2022 20:29
@TheJeterLP
Copy link
Contributor Author

@TheJeterLP why you add "text_display_ad_with_themed_cta_banner_image"?
It should be already blocked by "ad_" generic reference (placed at the top of list).
It's checked by a contains istruction, should be good already. 🤷‍♂️

Was just for debugging. Forgot to remove it from commit.

Anyway...have you understand how to block new elements? Was not much difficult at the end xD.

Yes, also implemented a log for that.

@TheJeterLP TheJeterLP merged commit 0476299 into ReVanced:main Jul 5, 2022
@ghost
Copy link

ghost commented Jul 5, 2022

@TheJeterLP why you add "text_display_ad_with_themed_cta_banner_image"?
It should be already blocked by "ad_" generic reference (placed at the top of list).
It's checked by a contains istruction, should be good already. 🤷‍♂️

Was just for debugging. Forgot to remove it from commit.

Anyway...have you understand how to block new elements? Was not much difficult at the end xD.

Yes, also implemented a log for that.

Good. This will be useful for future (eventually) new ads. 👍

@TheJeterLP TheJeterLP deleted the move-settings branch July 5, 2022 20:36
@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2022

🎉 This PR is included in version 0.21.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

None yet

1 participant