68209 2011-09-15 19:16:05 -0700 [EFL] Change WebKit EFL coding style 2011-12-29 19:36:03 -0800 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 68231 68321 68867 69073 69988 70883 71433 74474 1 gyuyoung.kim gyuyoung.kim donggwan.kim enmi.lee gyuyoung.kim kenneth leandro lucas.de.marchi rakuco sangseok.lim s.choi oldest_to_newest 468242 0 gyuyoung.kim 2011-09-15 19:16:05 -0700 WebKit EFL port has used EFL coding style. However, EFL port has used C-coding style despite files are C++. In addition, this style disturbed reviewers to review EFL patches. So, we discussed this issues by means of webkit-efl mailing list. https://lists.webkit.org/pipermail/webkit-efl/2011-September/000049.html I list current issues up as below. And, this bug is a meta bug to track this issues. 1. pointer operator coding style based on efl coding style. : pointer('*') operator coding style follows efl coding style. It looks we need to adhere webkit coding style. 2. Should we use EINA_TRUE and EINA_FALSE? : It can be replace with true / false. 3. Internal functions are using '(void)' parameter. : As we already discussed, internal function needs to use C++ coding style. 4. Manual memory management. : ewk_xxx files are using malloc, calloc and realloc instead of smart pointer. For example, GOwnPtr. 5. C style casting operator. : Though file format is .cpp, some codes are still using c type casting operator. 6. Meaningless parameter name. : We have used coding style of efl parameter so far. However, that is not webkit coding style. I heard that port implementation tends to be platform coding style. So, I added a parameter coding style exception to style checker for efl port. But, now I'm not sure whether we're able to use it continually. 469075 1 lucas.de.marchi 2011-09-17 09:08:50 -0700 (In reply to comment #0) > WebKit EFL port has used EFL coding style. However, EFL port has used C-coding style despite files are C++. In addition, this style disturbed reviewers to review EFL patches. So, we discussed this issues by means of webkit-efl mailing list. https://lists.webkit.org/pipermail/webkit-efl/2011-September/000049.html > > I list current issues up as below. And, this bug is a meta bug to track this issues. > > 1. pointer operator coding style based on efl coding style. > : pointer('*') operator coding style follows efl coding style. It looks we need to adhere webkit coding style. Ok, on bug 68231 I sent an initial configuration for uncrustify to fix this. > > 2. Should we use EINA_TRUE and EINA_FALSE? > : It can be replace with true / false. The problem here is that in public functions we must use Eina_Bool type. It's somewhat automatic then in those functions to use EINA_TRUE/EINA_FALSE. Internal functions should be fine using bool and thus true/false. For public functions, I'm not sure what approach we should take. > 3. Internal functions are using '(void)' parameter. > : As we already discussed, internal function needs to use C++ coding style. Agreed. > 4. Manual memory management. > : ewk_xxx files are using malloc, calloc and realloc instead of smart pointer. > For example, GOwnPtr. These are more difficult to handle. Recently Raphael Kubo converted some of them. > 5. C style casting operator. > : Though file format is .cpp, some codes are still using c type casting operator. This is another easy one. Since most of the time we are converting C structs or native types, almost all of them will be converted to static_cast<>. We could even automatically convert all of them to static_cast<> and going back if we need another type of cast. > 6. Meaningless parameter name. > : We have used coding style of efl parameter so far. However, that is not webkit coding style. > I heard that port implementation tends to be platform coding style. So, I added a parameter coding style exception to > style checker for efl port. But, now I'm not sure whether we're able to use it continually. Some of them are common language to EFL committers/users. I don't think changing it is a good idea, but if it's required we can use sed/coccinelle for this task. 469079 2 kenneth 2011-09-17 10:41:08 -0700 > > 2. Should we use EINA_TRUE and EINA_FALSE? > > : It can be replace with true / false. > > The problem here is that in public functions we must use Eina_Bool type. It's somewhat automatic then in those functions to use EINA_TRUE/EINA_FALSE. For return values this should be fine. 469081 3 rakuco 2011-09-17 12:11:27 -0700 (In reply to comment #2) > > > 2. Should we use EINA_TRUE and EINA_FALSE? > > > : It can be replace with true / false. > > > > The problem here is that in public functions we must use Eina_Bool type. It's somewhat automatic then in those functions to use EINA_TRUE/EINA_FALSE. > > For return values this should be fine. Eina_Bool Foo::bar() { // ... return true; } should work just fine too, there's just an implicit bool -> unsigned char conversion involved. 469229 4 gyuyoung.kim 2011-09-18 21:12:32 -0700 (In reply to comment #3) > Eina_Bool Foo::bar() > { > // ... > return true; > } > Can this conversion to reduce readability or make confusion to ewk contributor ? I think it is better to use only EINA_BOOL in public functions. 512702 5 rakuco 2011-12-01 05:13:41 -0800 All the bugs this report depends on have been closed, can we close this one too? 520638 6 gyuyoung.kim 2011-12-13 20:32:15 -0800 (In reply to comment #5) > All the bugs this report depends on have been closed, can we close this one too? I'd like to close this bug after Bug 74474 is landed. Because, that bug is also similar bug. 521676 7 gyuyoung.kim 2011-12-14 22:40:01 -0800 All coding style issues mentioned by this bug are closed. So, I close this bug.