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 class #48

Closed
wants to merge 9 commits into from
Closed

refactor: move settings to SettingsEnum class #48

wants to merge 9 commits into from

Conversation

TheJeterLP
Copy link
Contributor

@TheJeterLP TheJeterLP commented Jul 3, 2022

See #37

@TheJeterLP TheJeterLP mentioned this pull request Jul 3, 2022
@ghost
Copy link

ghost commented Jul 3, 2022

Based on @oSumAtrIX decision, this litho block should be removed because it disable comments (even if I would like it to keep):

private static boolean isExperimentalCommentsRemoval() {
        return getBoolean("experimental_comments", false);
}

@oSumAtrIX
Copy link
Member

No, it should not be removed.

@TheJeterLP
Copy link
Contributor Author

Based on @oSumAtrIX decision, this litho block should be removed because it disable comments (even if I would like it to keep it):

private static boolean isExperimentalCommentsRemoval() {
        return getBoolean("experimental_comments", false);
}

Makes no sense to remove a feature when theres a setting for it to toggle it.

@TheJeterLP TheJeterLP self-assigned this Jul 3, 2022
@ghost
Copy link

ghost commented Jul 3, 2022

Makes no sense to remove a feature when theres a setting for it to toggle it.

I agree. Maybe he re-evaluated his decision.

@oSumAtrIX
Copy link
Member

But I never said to remove it

@ghost
Copy link

ghost commented Jul 3, 2022

But I never said to remove it

Troll?
Immagine 2022-07-03 223922

@oSumAtrIX
Copy link
Member

oSumAtrIX commented Jul 3, 2022

The image is out of context. This was my next response (missing the "not" in the sentence):
image

@ghost
Copy link

ghost commented Jul 3, 2022

The image is out of context. This was my next response (missing the "not" in the sentence): image

Sorry, I hadn't paid attention to this comment.👍

Anyway, to close this OT, I'll push some other stuff to hide other comments.

@oSumAtrIX
Copy link
Member

Create a patch suggestion on the ReVanced organization discussion page, and see if people need it, before adding it. If you insist, simply make it optional and disabled by default.

@ghost
Copy link

ghost commented Jul 3, 2022

Create a patch suggestion on the ReVanced organization discussion page, and see if people need it, before adding it. If you insist, simply make it optional and disabled by default.

Again sorry.

@oSumAtrIX
Copy link
Member

Create a patch suggestion on the ReVanced organization discussion page, and see if people need it, before adding it. If you insist, simply make it optional and disabled by default.

Again sorry.

No worries :) Thanks for contributing!

@TheJeterLP TheJeterLP closed this Jul 4, 2022
@TheJeterLP TheJeterLP deleted the dev branch July 4, 2022 05:57
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

2 participants