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 2020/04/13 15:56:09 UTC

[GitHub] [fineract] thesmallstar opened a new pull request #758: Added Maximum Line length check and Enforced checkstyle

thesmallstar opened a new pull request #758: Added Maximum Line length check and Enforced checkstyle
URL: https://github.com/apache/fineract/pull/758
 
 
   ## Description
   Although the tools helped, after a lot of manual work I am left with 45 errors.
   These errors are mostly due to:
   1. Extremely! extremely! long variable names. 
   2. Hard coded strings.
   Possible solution -> 
   1. I replace all occurrence of this variable/function to something smaller.
   2.  Replace hardcoded strings(mostly in tests asserts) with pre defined strings.
   
   If the suggestions look good, I will make those changes for the build to pass :) 
   
   
   ## Checklist
   Please make sure these boxes are checked before submitting your pull request - thanks!
   
   - [ ] Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
   
   - [ ] Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
   
   - [ ] API documentation at https://github.com/apache/fineract/blob/develop/api-docs/apiLive.htm has been updated with details of any API changes.
   
   - [ ] Integration tests have been created/updated for verifying the changes made.
   
   - [ ] All Integrations tests are passing with the new commits.
   
   - [ ] Submission is not a "code dump".  (Large changes can be made "in repository" via a branch.  Ask on the list.)
   
   Our guidelines for code reviews is 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fineract] thesmallstar commented on issue #758: Added Maximum Line length check and Enforced checkstyle

Posted by GitBox <gi...@apache.org>.
thesmallstar commented on issue #758: Added Maximum Line length check and Enforced checkstyle
URL: https://github.com/apache/fineract/pull/758#issuecomment-613006254
 
 
   Checkstyle probably was not scanning the entire code base, It started showing another 1529 files that need to be fixed :P, this PR has a long way to go I guess :) 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fineract] vorburger commented on issue #758: Added Maximum Line length check and Enforced checkstyle

Posted by GitBox <gi...@apache.org>.
vorburger commented on issue #758: Added Maximum Line length check and Enforced checkstyle
URL: https://github.com/apache/fineract/pull/758#issuecomment-615402955
 
 
   @thesmallstar this has merge conflicts following #764 and #761 ...

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fineract] vorburger commented on issue #758: Added Maximum Line length check and Enforced checkstyle

Posted by GitBox <gi...@apache.org>.
vorburger commented on issue #758: Added Maximum Line length check and Enforced checkstyle
URL: https://github.com/apache/fineract/pull/758#issuecomment-613038548
 
 
   May be this particular check (LineLength) is one to tackle later - I would try to first identify those which you can enable with the least effort (so those that either have few, or many but easily fixable by IDE automation).

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [fineract] vorburger commented on issue #758: Added Maximum Line length check and Enforced checkstyle

Posted by GitBox <gi...@apache.org>.
vorburger commented on issue #758: Added Maximum Line length check and Enforced checkstyle
URL: https://github.com/apache/fineract/pull/758#issuecomment-612966581
 
 
   > If the suggestions look good, I will make those changes for the build to pass :)
   
   Sure, go for it!
   
   I'm wondering how, once the build passes, we'll most efficiently review this PR... several of us having a look? Just "trust" you, once all tests pass, that this PR is just reformatting without any functional changing? That's probably OK, for a change like 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services