> Hi Rostislav, > > Thanks for the patch! A few thoughts: > > - this mixes formatting, style, and functional changes, which should be > avoided.
The formatting and style changes are minimal. I just don't like mixing several formatting styles in the same unit of code. And if you look at my changes of the the BaseL10n class you could see that I've changed only one method - addHTMLSubstitutions(). This change is quite small, even if you take formatting into account. Also instead of looking at the patch file you can use a comparison software that usually shows the changes by a more handy way and also may have several modes, including modes that ignore whitespaces and other unimportant differences. But If you want I can explain each line of my changes in the addHTMLSubstitutions(). method. Other java class that was changed is BaseL10nTest and there is no formatting changes. All changes are "painfully obvious" and this is a test class. All other changes relate to properties files that were just renamed without any content change. > - the commit message should enumerate the behavior changes and explain > why they were made if it's not painfully obvious. The commit message was written for myself. I just used 'git format-patch' instead of 'git diff' to make the patch. Since I use Maven and all code was rearranged according to the Maven directories structure my patch probably can't be applied to the original Fred right away. At least that wasn't my intention. I just wanted to show what I've changed in the code extracted from the original Fred. And again, I can explain each code line I've changed. > - after 1471 is released we're planning to move to Gradle (and use > gradle-witness), but the directory structure changes will be > equivalent. At that point we will certainly merge something similar > to your changes (if not your changes outright) to restore sanity to > the BaseL10nTest resource path stuff. Why not to do so now? At least in the "next" branch that is what you use instead of the "master" one. > We usually do code review on GitHub; could you please open this as a > pull request? [0] > > - Steve > > [0] https://github.com/freenet/fred/pulls I never did github pull requests before. Do I need a github account? Do I need to make another patch that is compatible to the Ant directories structure used in Fred? Why the bug report with my original patch isn't enough? _______________________________________________ Devl mailing list [email protected] https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
