On 02/01/16 16:24, Rostislav Krasny wrote: >> 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? Yes. > Do I need to make another patch that is compatible to the Ant > directories structure used in Fred? Yes.
> Why the bug report with my > original patch isn't enough? Because the release manager is an overworked volunteer. Please stick to the project standards to make life easier for volunteers dealing with your contributions. Thanks. https://wiki.freenetproject.org/Coding_standards
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Devl mailing list [email protected] https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
