Hey Isaac, I'm reviewing your pull requests bit-by-bit, but I'm not going to be able to get through them all today, because I've got some other stuff I have to work on.
What I've seen so far looks great though, so I've been merging them, once I've tested the build on my local machine. One tiny niggle that I didn't feel was worth holding up the merge for: the ext.name and ext.year variables aren't necessary anymore because the header you're using doesn't address them. Those two lines can eventually be deleted, even if they aren't hurting anyone now. Best Regards, Myrle On Thu, Mar 1, 2018 at 8:38 PM, Isaac Kamga <[email protected]> wrote: > Thanks a million Myrle and Phil. > > I followed your instructions and have updated the Pull Request with a new > commit. > > At Your Service, > Isaac Kamga. > > On Thu, Mar 1, 2018 at 7:49 PM, Myrle Krantz <[email protected]> wrote: > >> Oops. Do what Phil says. He knows better. >> >> Greets, >> Myrle >> >> On Thu 1. Mar 2018 at 19:43 Phil Steitz <[email protected]> wrote: >> >> > On 3/1/18 10:34 AM, Myrle Krantz wrote: >> > > Hey Isaac, >> > > >> > > At a first glance, it looks good. May I ask though why you removed >> > > the license parameter strictCheck? >> > > >> > > Regards, >> > > Myrle >> > >> > The copyright statement should be removed entirely from the source >> > header files and placed instead in NOTICE.txt. >> > >> > See http://www.apache.org/legal/src-headers.html >> > >> > Phil >> > > >> > > On Thu, Mar 1, 2018 at 5:27 PM, Isaac Kamga <[email protected]> >> > wrote: >> > >> Hi Myrle, >> > >> >> > >> I've just updated copyright information in fineract-cn-lang and >> created >> > a >> > >> new pull request <https://github.com/apache/fineract-cn-lang/pull/4>. >> > >> >> > >> I patiently await your review and possible merger. >> > >> >> > >> At Your Service, >> > >> Isaac Kamga. >> > >> >> > >> On Thu, Mar 1, 2018 at 5:05 PM, Isaac Kamga <[email protected]> >> > wrote: >> > >> >> > >>> Hi Myrle, >> > >>> >> > >>> Thanks a million for your advice and guidance on this. >> > >>> >> > >>> I've closed the Pull Request, will do appropriate changes and send in >> > >>> another for review. >> > >>> >> > >>> At Your Service, >> > >>> Isaac Kamga. >> > >>> >> > >>> On Thu, Mar 1, 2018 at 4:56 PM, Myrle Krantz <[email protected]> >> wrote: >> > >>> >> > >>>> Hey Isaac, >> > >>>> >> > >>>> Replies inline: >> > >>>> >> > >>>> On Thu, Mar 1, 2018 at 1:20 PM, Isaac Kamga <[email protected]> >> > >>>> wrote: >> > >>>>> I have just updated the copyright information and package name on >> the >> > >>>>> fineract-cn-lang repository and sent in another pull request >> > >>>>> <https://github.com/apache/fineract-cn-lang/pull/3> for review. >> > >>>> Please make a pull request with *just* the copyright information >> > >>>> adjusted. Changing package names is a backwards incompatible >> change. >> > >>>> By changing them, you break everything that depends on lang. And >> all >> > >>>> of the other fineract cn repositories depend on lang. Package names >> > >>>> will have to be changed in all the repositories at once. >> > >>>> >> > >>>> We will have to continue to be careful about backwards compatible >> > >>>> changes until we have three things: >> > >>>> * signature checking >> > >>>> * our artifacts in an artifactory >> > >>>> * an established process of incrementing versions. >> > >>>> >> > >>>> Because changing package names is a global change and changing >> > >>>> copyright information can be done locally, one repository at a >> time, I >> > >>>> believe we should adjust the copyright information first. >> > >>>> >> > >>>>> On a related note, I'd like to ask experienced developers on >> Fineract >> > >>>> CN if >> > >>>>> it would be necessary to change the project's name ( from *lang* to >> > >>>>> *fineract-cn-lang* ) in the settings.gradle >> > >>>>> <https://github.com/apache/fineract-cn-lang/blob/develop/set >> > >>>> tings.gradle> >> > >>>>> file. >> > >>>> I don't see any reason to change those names. the project's name is >> > >>>> appended to the artifact id. If we did this, the artifact id in >> this >> > >>>> case would be org.apache.fineract.cn.fineract-cn-lang. It would >> > >>>> contain duplicated information. Of course we could adjust the build >> > >>>> elsewhere to change the artifact id back to lang. But i don't see a >> > >>>> benefit in changing the project name. >> > >>>> >> > >>>> But perhaps I'm missing something here? >> > >>>> >> > >>>>> Also, in the bintray.pkg section of the build.gradle file >> > >>>>> < >> > https://github.com/apache/fineract-cn-lang/blob/develop/build.gradle>, >> > >>>>> should the `repo` , `userOrg` and `vcsUrl` variables be changed >> too ? >> > >>>> These >> > >>>>> will help with subsequent updates to fineract-cn-api, >> > >>>>> fineract-cn-cassandra, etc. >> > >>>> Oops! Good catch. We can delete the bintray section in that >> > >>>> build.gradlel file until we're ready to introduce the use of the >> > >>>> apache artifactory. I don't think you'll find a section like that >> in >> > >>>> any other fineract cn project. I was experimenting there. >> > >>>> >> > >>>> Thank you for taking this on Isaac, >> > >>>> Myrle >> > >>>> >> > >>> >> > >> > >>
