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 CLI setup script, add prompts for muting and skipping native youtube ads #150

Merged
merged 5 commits into from
Jun 21, 2024

Conversation

ryankupk
Copy link
Contributor

@ryankupk ryankupk commented May 3, 2024

RE: #149 - the CLI setup wizard only sets the skip_ads and mute_ads config options to false and does not give any option to set them to true during the setup process. This change adds some minor refactorings for some things in the script like using a bespoke function for prompting the user for y/n, as well as adds prompts for skipping/muting native ads and saves the choice to the config.

@dmunozv04
Copy link
Owner

Thanks for the contribution to bring feature parity between both config editors! I'll make sure to review it when I have time

@ryankupk
Copy link
Contributor Author

ryankupk commented May 4, 2024

Thanks @dmunozv04. I'm not married to any line here, and I know that pulling the strings out to constants can be controversial. I'm open to any feedback.

I also saw that github is reporting every line as different but locally the git diff is a lot cleaner. I'm not sure if maybe it's a linefeed thing, I don't think that it's tab width or anything like that. I can look deeper if you're concerned about that at all.

@dmunozv04
Copy link
Owner

It does look cleaner this way. I merged new changes into the branch

@dmunozv04
Copy link
Owner

@ryankupk merged!

@dmunozv04 dmunozv04 merged commit 205191f into dmunozv04:main Jun 21, 2024
2 of 3 checks 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