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: partially invalid exception thrown #97

Merged
merged 1 commit into from
Aug 9, 2021

Conversation

LiuXing-R
Copy link
Contributor

1, Remove useless PolicyException
2, The PolicyException thrown is meaningful after the custom policy parameter is judged null

@davewichers
Copy link
Collaborator

@spassarop - what do you think of this proposed change?

@spassarop
Copy link
Collaborator

Changes make sense. The throws in constructors were already useless, the null policy exception moved a level below is still valid and maybe cleaner (avoids repeating the throw).

The one change I'm not sure is the main method on AntiSamyDOMScanner and not because it's useful, it's clearly not. But for if there is a real reason for an empty main on the project. @davewichers, is there any reason for en empty main to exist? Something about compulsary executable class for the Maven pipeline or whatever? If it can be deleted then the PR is all right.

@LiuXing-R
Copy link
Contributor Author

Two useless throws were missed.

@LiuXing-R
Copy link
Contributor Author

@davewichers
I've found that Antisamy has a lot of redundant code, and I'd be happy to refactor it if you agree.

@davewichers
Copy link
Collaborator

@spassarop - I doubt the main() is needed. @LiuXing-R - I'm going to accept this pull request. If you want to work on refactoring to clean up the code, feel free. Thanks!

@davewichers davewichers merged commit f109ea4 into nahsra:main Aug 9, 2021
@LiuXing-R LiuXing-R deleted the useless-exception branch August 11, 2021 07:15
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