Style Guide for Core apps

Bug #1315796 reported by Bartosz Kosiorek on 2014-05-04
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Dropping Letters
Undecided
Unassigned
Sudoku App
Undecided
Unassigned
Ubuntu Calculator App
Low
Unassigned
Ubuntu Calendar App
Wishlist
Unassigned
Ubuntu Clock App
Low
Unassigned
Ubuntu Music App
Undecided
Unassigned
Ubuntu Terminal App
Undecided
Unassigned
Ubuntu UI Toolkit
Triaged
Low
Unassigned
Ubuntu Weather App
Undecided
Unassigned
ubuntu-ui-toolkit (Ubuntu)
Low
Unassigned

Bug Description

WHAT As I developer, I want all code in any code-base should look like a single person typed it, no matter how many people contributed.

WHY Currently even within one application (for example ubuntu-calculator-app), the coding style is mixed. I causes that it is hard to read the code. Additional information: http://addyosmani.com/blog/javascript-style-guides-and-beautifiers/

HOW
1. Create or apply existing code style for Ubuntu SDK (example:
   https://github.com/rwaldron/idiomatic.js/,
   http://contribute.jquery.org/style-guide/js/
   https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml)
2. Modify application's README file and add information what Coding style should be used.
3. During Review, it will be great to check also code style

Related branches

summary: - Apply common Code Style for all Core apps
+ Code Style for Core apps
description: updated
summary: - Code Style for Core apps
+ Common Code Style for all Core apps
description: updated
summary: - Common Code Style for all Core apps
+ Apply common Code Style for all Core apps
description: updated
description: updated
description: updated
Changed in music-app:
status: New → Incomplete
Victor Thompson (vthompson) wrote :

I need more information. I can see how scripts can easily do auto-indentation... but anything more detailed would be fairly difficult. Also, most developers *should* be using the auto indentation found within Qt Creator. When they don't, issues with indentation arise. I don't think we need scripts for this, I just think we need developers to be aware of how to use their IDE. If mandating style leads to failures similar to how python can mandate pep8 compliance... then I think we've failed to do our job as a welcoming community of app developers.

Sam Bull (dreamsorcerer) wrote :

Well, if you want to be a welcoming community, then you can't mandate the IDE either (I've written all my QML in gedit without auto-indent).

When I started writing Ubuntu apps, I was unsure what style to use, and couldn't find any style guides, which was annoying. Fortunately, I ended up with a style close to the one Bartosz linked. But, it would be nice to have found that when I first set out.

Consistent style makes a project easier to read and thus easier to maintain. Note the number one rule of PEP8 is consistency:
"A style guide is about consistency. Consistency with this style guide is important. Consistency within a project is more important. Consistency within one module or function is most important."

I've certainly written code in a non-standard (and sometimes uncomfortable) way to ensure the style is consistent with the project I was hacking on (like 8-width indents in nautilus, instead of spaces). Using my usual style would just make the code difficult to read as it doesn't fit in with the rest of the code.

I don't think it's too much to ask for people to have code follow a rough style that matches the project. Otherwise, you have contributors who add new features, then disappear. As the code gets more of these features in more and more differing styles, you get to the point where it becomes too difficult for the maintainers to understand the code and fix bugs. Getting frustrated, the maintainers may leave the project, and then you just end up with a mess of code with lots of bugs nobody's going to fix.

Maybe that seems a little extreme, but I can see that happening if you get lots of drive-by contributions in varying styles. Certainly, for my projects, I won't accept a merge proposal unless it fits the style of the project, or I'm willing to restructure the code myself before merging it (which I'll do if it's fairly well structured already).

In summary, I believe this is a choice between short-term gain and long-term gain. You can be open to everyone and accept any patch if it simply works, or you can think about the long-term contributors/maintainers that will be looking after your projects for years to come.

In the latter case, if a patch fails to match the style of the project, thank the contributor and then kindly ask them if they can restructure it to a consistent style before it's merged. Most contributors are excited to get their code accepted into a project, and will happily do this one little extra task. Maybe you'll lose a few contributions, but it's likely someone else will implement the same feature further down the line and get it accepted. In the long term, you should then have a more stable project with happier maintainers.

Mihir Soni (mihirsoni) on 2014-05-09
Changed in ubuntu-calendar-app:
status: New → Triaged
importance: Undecided → Wishlist
Bartosz Kosiorek (gang65) wrote :

After investigation I think the most optimal is Google JavaScript Style Guide.
It is available at: https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml

There are many useful tips and recommendation how to write efficient, accurate beautiful code.

For code formatting they follow the C++ formatting rules in spirit:
https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Formatting

If we agree with some coding style, I think such information should be added into README file.

description: updated
Changed in ubuntu-calculator-app:
status: New → Triaged
importance: Undecided → Low
Bartosz Kosiorek (gang65) wrote :

Before reading Google Style Guides, click on the button: "Toggle all summaries"

description: updated
description: updated
Changed in ubuntu-clock-app:
status: New → Confirmed
importance: Undecided → Low
milestone: none → 2.2
Nekhelesh Ramananthan (nik90) wrote :

I agree with Sam Bull. I think it is reasonable to request new developers to follow a certain code style without increasing the barrier to contribution. As long as we are polite and guide to the appropriate resources (web links etc), I think we would still be welcoming. I used to use http://qt-project.org/doc/qt-4.8/qml-coding-conventions.html while coding the clock app. I will go through and see if there are other official QT code style that are defined and talk to the SDK devs if they have any thoughts on this. But certainly it would be beneficial if all the core apps follow the same code style.

Victor Thompson (vthompson) wrote :

There's an updated QML Convention Guide here: http://qt-project.org/doc/qt-5/qml-codingconventions.html

Additionally, some of this information has been added to the Core Apps Developer Guide: https://wiki.ubuntu.com/Touch/CoreApps/DevelopmentGuide

Bartosz Kosiorek (gang65) wrote :

Thanks for comments.

For me http://qt-project.org/doc/qt-5/qml-codingconventions.html it is draf for formatting style and have many mistakes:

1. Why in following lines are semicolons:
        if (photoImage.width > 200) {
            photoImage.width;
        } else {
            200;
        }
Does it recommend to use it?
Google link about semicolons: https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml#Semicolons

2. Why function is formatted as:
    function doSomething(x) // javascript functions
    {
     ...
and not like
    function doSomething(x) { // javascript functions
Google link: https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Spaces_vs._Tabs#Function_Declarations_and_Definitions

3. How we should format conditions?
Google link: https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Conditionals#Conditionals

4. Should we use space, tabs or both?
Google link: https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Spaces_vs._Tabs#Spaces_vs._Tabs

5. etc.

Here is article about different JavaScript coding style:
http://addyosmani.com/blog/javascript-style-guides-and-beautifiers/

description: updated
description: updated
description: updated
description: updated
summary: - Apply common Code Style for all Core apps
+ Style Guide for Core apps
Riccardo Padovani (rpadovani) wrote :

Well, I agree with Bartosz to have a general guideline and try to follow it, but I think we haven't to be too strictly, especially with new guys.

I like more the Google guideline, I think is more consistent and probably more widespread.

Changed in ubuntu-clock-app:
status: Confirmed → Fix Released
milestone: 2.2 → rtm
Zsombor Egri (zsombi) on 2014-10-01
Changed in ubuntu-ui-toolkit:
status: New → Triaged
importance: Undecided → Low
Zoltan Balogh (bzoltan) on 2014-11-24
Changed in ubuntu-ui-toolkit (Ubuntu):
importance: Undecided → Low
status: New → Triaged
Changed in ubuntu-calculator-app:
status: Triaged → Fix Committed
Changed in music-app:
status: Incomplete → New
Changed in ubuntu-calculator-app:
status: Fix Committed → Fix Released
Changed in ubuntu-terminal-app:
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers