Summary: | [EFL] Change WebKit EFL coding style | ||
---|---|---|---|
Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> |
Component: | WebKit EFL | Assignee: | Gyuyoung Kim <gyuyoung.kim> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | donggwan.kim, enmi.lee, gyuyoung.kim, kenneth, leandro, lucas.de.marchi, rakuco, sangseok.lim, s.choi |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Bug Depends on: | 68231, 68321, 68867, 69073, 69988, 70883, 71433, 74474 | ||
Bug Blocks: |
Description
Gyuyoung Kim
2011-09-15 19:16:05 PDT
(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.
> > 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.
(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. (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. All the bugs this report depends on have been closed, can we close this one too? (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. All coding style issues mentioned by this bug are closed. So, I close this bug. |