DoNotApprove: Gradle, Parameters, and Paging #237
Conversation
This file contains 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
kazetsukaimiko
commented
Sep 20, 2018
| public List<IssueType> getIssueTypes() throws JiraException { | ||
| return getIssueTypes(false); | ||
| } | ||
|
|
||
| /** | ||
| * Obtains the list of all issue types in Jira. | ||
| * @return all issue types | ||
| * @throws JiraException failed to obtain the issue type list. |
There was a problem hiding this comment.
This should be taking Parameter... parameters instead of a boolean. Hmm.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I'd like to get your thoughts on the following modifications and perhaps get them merged at some point. This PR has a few features of note:
Gradle Support
Builds and tests run via gradle. Several libraries required upgrade- mockito behaviors have changes (see tests) and the Java target/source version may be too high for the upstream author's liking (also seems to cause different date behaviors, still investigating). The project can still be built with Maven. The Gradle support allows me to reference this codebase as an out-of-project module to build instead of a Maven artifact that needs to be pushed to a repository first.
Auto-pagination on Agile
If a board has more than (50/ page size) stories, your client lib currently has no way of retrieving them. This modification automatically submits paginated follow-ups until the entire dataset has been retrieved, see "Parameters" below. Example: If a board has 250 sprints and your page size is 10 items, 25 requests will be made when you call board.getSprints().
This currently is done using Stream.concat() and recursion, and preserves the order from paging. Ideally the method signature should change to Stream so that the paging can be invoked lazily as it is being requested (upon collection), but that's a JDK version question.
To get "extra" fields in JIRA, one needs to pass expand=field to the service at the request time. For me, I needed the changelog field on stories. These changes preserve the current API contract by using varargs(Parameter...) which is kinda nifty if I may say so myself. This one I think is pretty solid for cherry-picking.
Problems I'm currently solving:
Dates seem to be several hours off with the JDK version switch, investigating.
Other ideas:
I think switching from JodaTime to the java.time.* would be wise. You may be considering older JDKs, which is fine. Maven/Gradle no longer support 1.5 and the codebase could be cleaned up quite dramatically with things like Streams and Optionals. I wouldn't want to invest in a refactor effort without knowing how receptive upstream would be... as that would determine in large part how and what I'd choose to refactor.