You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2022/07/18 10:59:08 UTC
[GitHub] [fineract] BrianMuhimbura opened a new pull request, #2434: Muhimbura Brian Assesment
BrianMuhimbura opened a new pull request, #2434:
URL: https://github.com/apache/fineract/pull/2434
## Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
galovics commented on PR #2434:
URL: https://github.com/apache/fineract/pull/2434#issuecomment-1206126083
No reaction after ~20 days, closing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2434:
URL: https://github.com/apache/fineract/pull/2434#discussion_r938510773
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -6,9 +6,9 @@
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ * <p>
Review Comment:
Please remove these <p> tags
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2434:
URL: https://github.com/apache/fineract/pull/2434#discussion_r938511510
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/domain/Client.java:
##########
@@ -101,6 +102,9 @@ public final class Client extends AbstractPersistableCustom {
@Column(name = "lastname", length = 50, nullable = true)
private String lastname;
+ @Column(name = "surname", length = 50, nullable = true)
+ private String surname;
Review Comment:
Why do we need surname? We already have lastname field
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics closed pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
galovics closed pull request #2434: Muhimbura Brian Assesment
URL: https://github.com/apache/fineract/pull/2434
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2434:
URL: https://github.com/apache/fineract/pull/2434#discussion_r938514169
##########
fineract-provider/src/main/resources/db/changelog/tenant/changelog-tenant.xml:
##########
@@ -39,4 +39,5 @@
<include file="parts/0017_fix_stretchy_reports.xml" relativeToChangelogFile="true"/>
<include file="parts/0018_pentaho_reports_to_table.xml" relativeToChangelogFile="true"/>
<include file="parts/0019_refactor_loan_transaction.xml" relativeToChangelogFile="true"/>
+ <include file="parts/0020_MyInterviewMigration.xml" relativeToChangelogFile="true"/>
Review Comment:
Interview?? is this an interview task?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
galovics commented on PR #2434:
URL: https://github.com/apache/fineract/pull/2434#issuecomment-1198014556
Please update your PR with the latest develop changes and make sure you use rebase (I see you did a merge).
Also, make sure you're squashing your commits for the PRs. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on a diff in pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
galovics commented on code in PR #2434:
URL: https://github.com/apache/fineract/pull/2434#discussion_r932104715
##########
fineract-db/V1__MyInterviewMigration.sql:
##########
@@ -0,0 +1,6 @@
+ALTER TABLE `m_clients`
Review Comment:
This is definitely wrong cause Fineract is using Liquibase, not flyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] BrianMuhimbura commented on a diff in pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
BrianMuhimbura commented on code in PR #2434:
URL: https://github.com/apache/fineract/pull/2434#discussion_r938518539
##########
fineract-provider/src/main/resources/db/changelog/tenant/changelog-tenant.xml:
##########
@@ -39,4 +39,5 @@
<include file="parts/0017_fix_stretchy_reports.xml" relativeToChangelogFile="true"/>
<include file="parts/0018_pentaho_reports_to_table.xml" relativeToChangelogFile="true"/>
<include file="parts/0019_refactor_loan_transaction.xml" relativeToChangelogFile="true"/>
+ <include file="parts/0020_MyInterviewMigration.xml" relativeToChangelogFile="true"/>
Review Comment:
Hello @adamsaghy this PR was an interview task
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2434:
URL: https://github.com/apache/fineract/pull/2434#discussion_r938513460
##########
fineract-provider/src/main/java/org/apache/fineract/useradministration/domain/AppUser.java:
##########
@@ -74,6 +74,9 @@ public class AppUser extends AbstractPersistableCustom implements PlatformUser {
@Column(name = "lastname", nullable = false, length = 100)
private String lastname;
+ @Column(name = "gender", nullable = false, length = 100)
+ private Integer gender;
Review Comment:
What is the reason to add gender to Appuser? Seems odd to me...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] galovics commented on pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
galovics commented on PR #2434:
URL: https://github.com/apache/fineract/pull/2434#issuecomment-1198020278
@BrianMuhimbura can you please link the related JIRA ticket to this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2434:
URL: https://github.com/apache/fineract/pull/2434#discussion_r938510011
##########
fineract-provider/build.gradle:
##########
@@ -1,4 +1,5 @@
-/**
+targetCompatibility = JavaVersion.VERSION_16
Review Comment:
Please remove this! In the top level build.gradle, it is already configured to marke compatibility to JavaVersion.VERSION_17
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [fineract] adamsaghy commented on a diff in pull request #2434: Muhimbura Brian Assesment
Posted by GitBox <gi...@apache.org>.
adamsaghy commented on code in PR #2434:
URL: https://github.com/apache/fineract/pull/2434#discussion_r938511099
##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/client/api/ClientApiConstants.java:
##########
@@ -85,6 +85,7 @@ public class ClientApiConstants {
public static final String firstnameParamName = "firstname";
public static final String middlenameParamName = "middlename";
public static final String lastnameParamName = "lastname";
+ public static final String surnameParamName = "surname";
Review Comment:
lastname and surname is equivalent, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org