libgdx
libgdx copied to clipboard
libgdx
Queue Testing and Quality improvement
In order to improve the test coverage of the code base of libGDX, this pull request is created.
In this pull request, the following has been done to the file utils/queue:
- Added/Extended the test coverage
- Removed commented code
- Updated/Improved the javaDocs
- Minor static analysis upgrades
The code coverage has been improved as follows
| File | Class, % | Method, % | Line, % |
|---|---|---|---|
utils/queue (old) |
100% | 70% | 80% |
utils/queue (new) |
100% | 100% | 100% |
@ColinGeukes hey, thank you for this PR every test is very welcome. My only concern with this PR is that there are lost of style changes. Do you mind using the lint command so we can review more easily?
@yuripourre Thank you very much for your input. I will apply the lint command in the next commit, hope that will fix the styling changes.
@yuripourre The libGDX formatter was applied to revert the style changes, only javadocs now start at a new line in the form of
/**
* JavaDoc
*/
previously the file used the format of:
/** JavaDoc */
Should I also revert that? It is not being applied by the formatter.
@mgsx-dev thank you very much with your input, it is my first merge request with an open source project, I am trying to adapt to the project.
Sorry to be a bit rude and i don't want to discourage you.
Your input really helps in delivering quality merge requests that fit the project.
Junits you added doesn't hurt though.
After this merge request I will mostly focus on the addition of test cases instead of refactoring the documentation.
but i think it'd be better to focus on the part of libGDX that really miss documentation and tests. And i don't think collections are at the top priority.
I understand that there is little gain in updating the text, so I am continuing with mainly improving the test coverage after this merge request. If an entire file lacks documentation I might update that as well.
Thank you so far for the input, I believe I have resolved
Do not add @Override, diamond operators, or other stylistic changes if they do not already exist in the class you are modifying.
Queue is already thoroughly javadoc'ed before this PR, at least by my own rules. Javadoc is only used when it is needed. Adding javadoc to something that is already clear is not helpful (eg "Sets the name." for a setName method). Using more words to say the same thing is not helpful. Specifying that a "valid" thing is created or returned is just fluff.
Tests are good, especially for collections. We could merge the Queue test, but not any of the changes to Queue (unless some useful change is hidden in the noise).
@NathanSweet I reverted ALL changes to the Queue.java. Even commented code is back.