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

Some small performance improvements. #22

Merged
merged 21 commits into from
Oct 31, 2018
Merged

Conversation

guidotrueb
Copy link
Collaborator

Replaced manual copy of the array with the faster native copy and replaced some string concatenation. Although compiler can create StringBuilder concatenation in some instances, it is not smart enough to do it in between loops/ifs or, when used inside other StringBuilders, it'll (most probably) create new ones.

@wltjr
Copy link
Collaborator

wltjr commented Oct 31, 2018

I need to update the readme as to such. Presently this project is not being actively developed, or maintained. I received information sometime back from others that maintained the project before as to why it became orphaned. It is not entirely orphaned here, but not sure anyone really to review or tests your changes. Because of such, if you are using jtidy and have interest. I can add you to the project so you can make changes as you see fit. Hopefully not doing anything malicious or that would break the project. But then again you likely would notice before most others as I am not sure many are using this project.

In a nutshell, happy to add you so you can commit directly and make changes as you see fit. I and others will remain. But really no one to review and approve such changes. @dellgreen may if he has spare time. But I think he has found another project or went a different direction and is not using jtidy. Up to you, just wanted to give you some insight and extend an offer for direct project involvement.

@guidotrueb
Copy link
Collaborator Author

I need to update the readme as to such. Presently this project is not being actively developed, or maintained. I received information sometime back from others that maintained the project before as to why it became orphaned. It is not entirely orphaned here, but not sure anyone really to review or tests your changes. Because of such, if you are using jtidy and have interest. I can add you to the project so you can make changes as you see fit. Hopefully not doing anything malicious or that would break the project. But then again you likely would notice before most others as I am not sure many are using this project.

In a nutshell, happy to add you so you can commit directly and make changes as you see fit. I and others will remain. But really no one to review and approve such changes. @dellgreen may if he has spare time. But I think he has found another project or went a different direction and is not using jtidy. Up to you, just wanted to give you some insight and extend an offer for direct project involvement.

I would love too. My company still uses JTidy and we need to patch some improvements, so I'm doing some clean-up and will probably merge some work because it is still relevant to us. Thank you very much for the opportunity. ;)

@wltjr
Copy link
Collaborator

wltjr commented Oct 31, 2018

Great, just sent invite. You have admin privileges, and can add others as you see fit. Please do not change the privileges of others, or remove anyone. This project was forked all over the place on Github. With forkers permissions we changed them all so this is the master for all other forks. Each of the owners of the previous forks were also invited. Thus all the people, despite the lack of current development or maintainers.

Till another shows interest or has some input. You are the head honcho, so you can do what ever with the code base as you see fit. Have fun! let me know if I can help or you have any questions, etc.

@guidotrueb
Copy link
Collaborator Author

guidotrueb commented Oct 31, 2018

Great, just sent invite. You have admin privileges, and can add others as you see fit. Please do not change the privileges of others, or remove anyone. This project was forked all over the place on Github. With forkers permissions we changed them all so this is the master for all other forks. Each of the owners of the previous forks were also invited. Thus all the people, despite the lack of current development or maintainers.

Till another shows interest or has some input. You are the head honcho, so you can do what ever with the code base as you see fit. Have fun! let me know if I can help or you have any questions, etc.

OK, don't worry. And thanks for your trust! One quick question: were all other forks merged into this one, including the "Java5" branch changes from the original SVN repository and spullara/jtidy, trajano/jtidy, geoffmcl/tidy-fork and gnosygnu/jtidy_xowa? If you're not sure, I can check that out.

@wltjr
Copy link
Collaborator

wltjr commented Oct 31, 2018

I believe so, I tried to grab any good stuff and merge it into this one. But I really cannot recall exactly what was done. It should be in git history. If something was not done, or you have other improvements. Feel free to do what ever as you see fit. I believe it can use some substantial improvements. I can forward you the email I received from a previous developer/maintainer when it was on Source Forge. that will give you some more details on what needs to be done, etc. The legacy code that could be improved.

@wltjr
Copy link
Collaborator

wltjr commented Oct 31, 2018

Just sent you a forwarded copy of an email between myself and a past project developer who gave me some insight to the project history. May end up in your spam/junk folder from "William L. Thomson Jr." [email protected]

@guidotrueb
Copy link
Collaborator Author

William L. Thomson Jr.

Thank you! That will be very helpful.

@wltjr
Copy link
Collaborator

wltjr commented Oct 31, 2018

FYI travis is failing due to Sonar, not your changes. Not sure what is up with Sonar. Hopefully just something broken for this project. I use it for a few. I haven't pushed to them in a bit, but they all worked last time I did. Seems to be something with connecting to the local instance. Once your done I will look into that.

Dell was working on fixing tests, but that is where he left off. Though the test failing do not cause the build to fail. The regular builds are passing in Travis. It is just the sonar one for some unrelated reason.

@guidotrueb
Copy link
Collaborator Author

guidotrueb commented Oct 31, 2018

FYI travis is failing due to Sonar, not your changes. Not sure what is up with Sonar. Hopefully just something broken for this project. I use it for a few. I haven't pushed to them in a bit, but they all worked last time I did. Seems to be something with connecting to the local instance. Once your done I will look into that.

Dell was working on fixing tests, but that is where he left off. Though the test failing do not cause the build to fail. The regular builds are passing in Travis. It is just the sonar one for some unrelated reason.

Yes, Travis is failing due to Sonar (it was running default mvn sonar:sonar but somehow it tried to connect to localhost instead of sonarcloud, I guess. That's why I changed to the sonar-scanner script. But anyway, I'll merge the code and see the quality gate if it passes now. Maybe sonar is failing just for this pull request automated review and it'll work fine once upstreamed. I'll work on some tests too, as I like to see them all green and generally they give good hints of bugs. :)

@guidotrueb
Copy link
Collaborator Author

Got it... It is some restriction for pull request builds, documented here travis-ci/travis-ci#10062 and here https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions.

@guidotrueb guidotrueb merged commit 2d0b46e into jtidy:master Oct 31, 2018
@dellgreen
Copy link
Collaborator

@guidotrueb its awesome that you can help out, @wltjr is correct i am stretched on other projects and unable to maintain this one presently, so thank you for joining the team :)

@guidotrueb
Copy link
Collaborator Author

Thank you @dellgreen ! The honor is mine. We'll keep in touch.

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