Open
Conversation
1. Suggest using [semver](https://semver.org/) for versioning. 2. A version at github shall always be a SNAPSHOT version. 3. The license license.url in your pom does not point to a BSD license as you said in license.name Just point it to your LICENSE file in github repo is a better choice. 4. Specify java.version in properties. 5. Specify source/target version in maven compiler plugin. 6. Add maven-site-plugin and maven-project-info-reports-plugin to make sure when you run `mvn site`, a javadoc will generate and link to the generated site. 7. Update maven dependencies to latest version. 8. Update other dependencies to latest version of same major version. 9. refine .gitignore for excluding .iml s and target folders.
1. in class ServletOutputStreamCopier: 1.1 field ByteArrayOutputStream copy shall be final. 1.2 public boolean isReady()infinite loop itself. 1.3 public void setWriteListener(WriteListener writeListener) infinite loop itself. 2. in class TemplateReplace: 2.1 Don't create a StringBuilder for just 2 or 3 params. Just use + will be a faster / neater choice 3. in class SonicUtil: 3.1 Don't return inside a try block. Inside return it afterwards. And, you didn't use the local String encryptText. Even if you intend to return from the try block, you shall return null directly after the catch block. Return in try block will always cause problem for maintainers so really not recommended to do so.
Member
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
add pom here and fix the pom.
Suggest using semver for versioning.
A version at github shall always be a SNAPSHOT version.
The license license.url in your pom does not point to a BSD license as you said in license.name
Just point it to your LICENSE file in github repo is a better choice.
Specify java.version in properties.
Specify source/target version in maven compiler plugin.
Add maven-site-plugin and maven-project-info-reports-plugin to make sure when you run
mvn site, a javadoc will generate and link to the generated site.Update maven dependencies to latest version.
Update other dependencies to latest version of same major version.
refine .gitignore for excluding .iml s and target folders.
fix bugs and malformed codes.
1. in class ServletOutputStreamCopier:
1.1 field ByteArrayOutputStream copy shall be final.
1.2 public boolean isReady()infinite loop itself.
1.3 public void setWriteListener(WriteListener writeListener) infinite loop itself.
2. in class TemplateReplace:
2.1 Don't create a StringBuilder for just 2 or 3 params. Just use + will be a faster / neater choice
3. in class SonicUtil:
3.1 Don't return inside a try block. Inside return it afterwards. And, you didn't use the local String encryptText. Even if you intend to return from the try block, you shall return null directly after the catch block.
Return in try block will always cause problem for maintainers so really not recommended to do so.
add missing @OverRide.
specify init capacity for HashMap for better performance.