-
-
Notifications
You must be signed in to change notification settings - Fork 12
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
#717 Started implementing GitlabIssues. #718
Conversation
@criske thank you for your Pull Request. I'll assign someone to review it soon. |
@@ -210,15 +209,12 @@ public double percentage() { | |||
|
|||
@Override | |||
public BigDecimal commission(final BigDecimal value) { | |||
final double twoDigitsPercentage = Double.valueOf( | |||
new DecimalFormat("0.00").format(this.percentage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amihaiemil On my end the test for this part was failling with
java.lang.NumberFormatException: For input string: "8,00"
due to using comma separator imposed by Locale
, most likely. So I've changed it to use BigDecimal
s only.
@amihaiemil please review this Pull Request. Deadline (when it should be merged or closed) is You should check if the requirements have been implemented (partially or in full), if there are unit tests covering the changes and if the CI build passes. Feel free to reject the PR or ask for changes if it's too big or not clear enough. Estimation here is |
return this.json | ||
.getJsonObject("references") | ||
.getString("full") | ||
.split("[#!]")[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amihaiemil we're using a regexed split cause
Issue json is using #
"references": {
"short": "#1",
"relative": "#1",
"full": "my-group/my-project#1"
}
Pull request (merge request) is using !
"references": {
"short": "!1",
"relative": "!1",
"full": "my-group/my-project!1"
}
|
||
@Override | ||
public String issueId() { | ||
return String.valueOf(this.json.getInt("iid")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opted for iid
(relative id to project) insted of id
(global id) because Issues and Pull Requests are using iid
in their url:
GET /projects/:id/issues/:issue_iid
GET /projects/:id/merge_requests/:merge_request_iid
- fixed test coverage for GitlabIssue.
@rultor merge it please |
@amihaiemil OK, I'll try to merge now. You can check the progress of the merge here |
@amihaiemil @criske Oops, I failed. You can see the full log here (spent 3min)
|
@criske some puzzle is not correct, can you fix it pls? :D |
@amihaiemil thank you for resolving this ticket. I've just added it to your active invoice. You can always check all your invoices and more on the Contributor Dashboard. |
closes #717