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

Make TerminalSolver interface & ExperimentSolver class sealed #722

Conversation

AzureAaron
Copy link
Collaborator

@AzureAaron AzureAaron commented May 19, 2024

Also made the ExperimentSolver class sealed.

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label May 19, 2024
@AzureAaron AzureAaron added this to the 1.21 milestone May 19, 2024
@AzureAaron AzureAaron changed the title Make TerminalSolver interface sealed Make TerminalSolver interface & ExperimentSolver class sealed May 20, 2024
@kevinthegreat1
Copy link
Collaborator

I don't get the point of making all the subclasses non-sealed. They shouldn't be extended anyways?

@AzureAaron
Copy link
Collaborator Author

I don't get the point of making all the subclasses non-sealed. They shouldn't be extended anyways?

Make them final instead?

@kevinthegreat1
Copy link
Collaborator

I didn't realize you have to put either non-sealed or final. That's really stupid and makes me think that we don't need this at all. But you can choose either non-sealed or final, I don't really care then.

@AzureAaron
Copy link
Collaborator Author

I didn't realize you have to put either non-sealed or final. That's really stupid and makes me think that we don't need this at all. But you can choose either non-sealed or final, I don't really care then.

I swapped them to final as they should not be extended.

@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels May 20, 2024
@kevinthegreat1 kevinthegreat1 merged commit 610c647 into SkyblockerMod:master May 20, 2024
1 check passed
@LifeIsAParadox LifeIsAParadox removed the merge me please Pull requests that are ready to merge label May 20, 2024
@AzureAaron AzureAaron deleted the sealed-interface-for-term-solvers branch May 22, 2024 23:58
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.

3 participants