-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 John Crispin <blo...@openwrt.org> wrote: > > > On 17/08/2015 23:25, Karl Palsson wrote: > > Personally I'd like to see Gainstrong mentioned _somewhere_ in the patch > > itself. > > > >> +--- a/arch/mips/ath79/machtypes.h > >> ++++ b/arch/mips/ath79/machtypes.h > >> +@@ -74,6 +74,7 @@ enum ath79_mach_type { > >> + ATH79_MACH_JA76PF2, /* jjPlus JA76PF2 */ > >> + ATH79_MACH_JWAP003, /* jjPlus JWAP003 */ > >> + ATH79_MACH_HORNET_UB, /* ALFA Networks Hornet-UB */ > >> ++ ATH79_MACH_MINIBOX_V1, /* MINIBOX V1.0 */ > > > > Its a comment, but feel free to send a patch to modify the comment.
And my review comment is that retyping the define is useless, most of the defines use the comment to include a full name, normally including the vendor, in this case Gainstrong. This entire patch has no mention of the word Gainstrong in it at all, just "minibox" in various capitalizations. Do you have a particular reason to _encourage_ omitting useful, common information? Is there a reason that this version as submitted would be _preferable_ to a version that followed the existing examples and included useful vendor information? Is there any reason that I would want to send a _patch_ to fix someone else's patch, that hasn't even been merged? > > > Maybe here? Otherwise that comment is pretty irrelevant... > > > >> + ATH79_MACH_MR12, /* Cisco Meraki MR12 */ > >> + ATH79_MACH_MR16, /* Cisco Meraki MR16 */ > >> + ATH79_MACH_MR600V2, /* OpenMesh MR600v2 */ > > > > The ALLCAPS_PREFIX_ in the board file is... special? Have you seen that > > anywhere else? > > -EPARSE > > - -EAGAIN? >> +static const char *MINIBOX_V1_part_probes[] = { >> +static struct flash_platform_data MINIBOX_V1_flash_data = { >> +static struct gpio_led MINIBOX_V1_leds_gpio[] __initdata = { >> +static struct gpio_keys_button MINIBOX_V1_gpio_keys[] __initdata = { >> +static void __init MINIBOX_V1_setup(void) I've never seen all caps used in the prefix of methods in board files. Truly, I'm not widely read, but that would seem to be very out of style. Is this a style you wish to encourage? If so, please let the community know so we can patch other files to bring them inline. Sincerely, Karl Palsson -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIcBAEBAgAGBQJV0wcWAAoJEBmotQ/U1cr2FUAQALqSPtknCawz8zqL5mfmjC4N ynsNs6y3YQ8ZOc6RVTMdCLTvtNZmE1NZcrk7nAKVzrZBpXDCB6j6iPIal7fkM55m +mrilQUwj2ZdFJ0Egfj4QFsdqc6LIth5WalpnkNqrnBsDEgUpUX6Vs+Xb8alJFhC Iv+ILHvEif2ejlhM7Z/k39EzGsnDxLHUBY/tyJoVtJbOisO4oL73PXigjpx8cdrY LcUoB3I84Pi2eLCo+iA2YOBioc3dR+ZY/lpSp+h0HF3XP8CYePsjbAa7H0LOJzC/ M44uXJoeGYQFSv23sVt6SZKFjFF6+rthMrf1WDLTH9NHU4venJsX/7Jn1XcVhcte 6QEZ60IYBTbXzw2GrN+p6dWYtGZia+blECXjhDCRqH6/72GdoD/XnVly4fH9M9HE ekSjBMCCd7P6AWCwzf1z1Kg9adUQWOaajwSFyW75psb9f3p44tU8bpt0d82/aFam +oCtcgjGoU6O7ndgH5QIYERIk9O65CZu90VQh5G2B1cYn1OE2ODoGMGr/yXNOWbs t01vrbDtKKmkkpuyXwtDIulq8sOQhrAw0O37c7bIgGsH1CYUipEFyTDw2ngSalsO 7V3xsIRHK4/JHI4wUi17kK9r1h+5FSvFQIkHZuEyDR6/mdZ+GsO4zCf+YTghRjaK bl+7Nsr+645uGFMuFfpb =8o2o -----END PGP SIGNATURE-----
_______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel