MrMEEE / bumblebee-Old-and-abbandoned

OUTDATED!!!!! - Replaced by "The Bumblebee Project" and "Ironhide"
http://www.martin-juhl.dk/2011/08/ironhide-reporting-for-duty/
469 stars 50 forks source link

Coding standards #554

Open Lekensteyn opened 13 years ago

Lekensteyn commented 13 years ago

Let's establish some coding standards as the current code base is getting horrific.

The current proposal is visible at: https://github.com/MrMEEE/bumblebee/wiki/Coding-Standards

Please discuss before changing.

Samsagax commented 13 years ago

Totally agree with this. It's a good way to make the code more readable. I'll also suggest some blank lines and a starting comment on what are we trying to acomplish withing a block:

Something like this:

# I'm saying hello
some code to raise my hand.
wave it
if wavedback; then
    say hello
fi

Also, for the record, I prefer 3 spaces instead of 4 for indentation, but is just a number in the tab substitution.

ArchangeGabriel commented 13 years ago

Also totally agree with Lekensteyn. Including 4 spaces for indentation.

And please when every one will be ok, let me correct actual code this way, I have a great idea on how to make it very fast. And as I am maniac, I will really enjoy it :P

Lekensteyn commented 13 years ago

sed? :p

Wiki created: https://github.com/MrMEEE/bumblebee/wiki/Coding-Standards Does everyone agree with it? About variable definition, should we allow:

local a=1 b=2

or must it be:

local a=1
local b=2

or even:

local a b
a=1
b=2

Another thing, I've always tried to keep the lines within 80 characters unless it became unreadable or such. Your opinions?

ArchangeGabriel commented 13 years ago

A friend of mine also do that. Originally, this is for printing purposes using vim.

But we can use it as a standard for readability.

For local, both first and second are convenient for me. So I let the others choose.

And no, not sed, as I'm newby to the Linux world and don't now what are the "basics". I would to that using N++ under windows.

Samsagax commented 13 years ago

For variables I'd prefer the second or third options for readability. For line lengths I always put one line per statement except in really short 'if' statements:

if condition; then; function_call; fi

That will always keep the lines under 80 chars unless some very large expression. In such cases a temporary variable would become in handy to break the function in smaller pieces.

ArchangeGabriel commented 13 years ago

So let's use the second for variables.

Lekensteyn commented 13 years ago

Included the 80 char limit, local a=x b=y deprecation and if condition; then something; fi. That could be shortened to condition && something. I prefer using that, is it forbidden to use such constructs, or am I allowed to?

ArchangeGabriel commented 13 years ago

Yes you can, I will do the same.

Ximi1970 commented 13 years ago

Just to be clear. Are you going to use a tab ( 4 spaces wide ) or are you going to use 4 spaces as indent? I hope you are not even thinking about the last option.....It really s*cks when trying to indent a selected block of text with [shift]--tab.

And it is a great idea to implement a standard. In our company we implemented it about 20 years ago. It is much easier for every one to read / understand the code and add some new stuff. One of our best rules: Keep the code spacy, use A LOT of spaces in your lines and please try to add some comments. So do not generate oneliners like this:

main(int c,char*_v){return!m(v[1],v[2]);}m(char_s,char_t){return_t-42?_s?63==_t|_s==_t&&m(s+1,t+1):!_t:m(s,t+1)||_s&&m(s+1,t);}

( for the sed command this is almost impossible to live by, sorry about that :-) )

ArchangeGabriel commented 13 years ago

In fact we were going to use 4 spaces, not a tab.

Because you say that a tab is 4 spaces, but that is almost false, the tab size or even encoding is system-related and even sometimes editor-related.

Not the space.

Ximi1970 commented 13 years ago

4 spaces was just an example for the size. Everybody can choose his own indent size when using tabs. Mine is 4 spaces wide. Somebody else can use 2 space wide tabs. It really does not matter what the other guy/girl is using as ident width. You will edit the file always with your OWN indentation. It is therefore unnecessary to define an ident size when tabs are used.

ArchangeGabriel commented 13 years ago

How do you set the tab size ?

And for information, when I edit something under Windows and use tab for indentation, they are replaced by 2 tabs under Linux...

Ximi1970 commented 13 years ago

In Windows: Visual Studio you can find it here: Tools->Options->Text Editor->All Languages->Tabs or Tools->Options->Text Editor->< the language you are using->Tabs UltraEdit: Advanced->Configuration->tab Edit

Linux: Kwrite - >Settings->Configure Editor->Editing->Tab Identation Qt creator->Tools->Options->Text Editor->Behavior

ArchangeGabriel commented 13 years ago

Ok !!

I wasn't using one of this programs but I found the equivalent. I've just realized I had never look at gedit options...

But as Lekensteyn said, there is an option that should help you : Indentation cluster: four spaces (not tabs, you can change the tab behavior on most editors to produce spaces when hitting TAB key)

And I confirm it works with MAJ-Tab.

Ximi1970 commented 13 years ago

Yes I know. They are going to all lengths to use spaces as tabs. Just WTF....

Lekensteyn commented 13 years ago

Most modern editors allow for transparent removal of indentation, so that pressing backspace takes four spaces indention away. Have your considered the other points in the standard? It's not limited to indention.

I've used tabs a while a go, but switched to spaces to get consistent display in different programs (editors, browser, printing)

Ximi1970 commented 13 years ago

I normally (for C,C++) use this syntax:

if( x > 0 ) then
{
    printf( "Hello" );
}

We do not allow people to use the short forms like this

if( x > 0 ) then printf( "Hello" );

because it can lead to bugs when people are adding code too fast and try this:

if( x > 0 ) then printf( "Hello" );
    printf( " world!" );

instead of

if( x > 0 ) then
{
    printf( "Hello" );
    printf( " world!" );
}

Putting the { and } both on the indent line will also make it more readable and define the block better.

And I really like spaces :-) so this:

for ((i=0; i<$length; i++)); do

should be:

for (( i = 0 ; i < $length ; i++ )) ; do
ArchangeGabriel commented 13 years ago

And does not render spaces.

But I can view the raw data of your post, and I agree with everything you've said in it. Including "I really like spaces".

If you really like them, why don't you use them as tabulation ? :P

Ximi1970 commented 13 years ago

Because the "old" editors I used did not have the space replace options......And if you want to indent files with that kind of editor, please be my guest. Especially when the files are very large...

Ximi1970 commented 13 years ago

Another syntax I am not going to recommend: putting the ';' on position 80 to mark the end of the lineprinters reach. Code beyond the 80 would not be printed and could not be seen when using the MSDOS debugger. :-)

Lekensteyn commented 13 years ago

@Ximi: github uses Markdown which was designed to be easy. Use four leading spaces to start a code block or use three backticks like:

code

I don't mind using tabs instead of spaces, as long as everyone sticks to it and do not mix those up.

My editor shows a line in the background for the X characters limit ;) (with X defaults to 80)

ArchangeGabriel commented 13 years ago

@Ximi1970 : we're not putting a ; on position 80, just going to the next line if able.

Ximi1970 commented 13 years ago

Just joking :-)

Ximi1970 commented 13 years ago

@Lekensteyn: Thanks for the info, case of RTFM Ximi1970. Never have time for reading manuals :-)

Yes that is the most important part: do not mix styles. Problem is: all my editors are tab "minded" for my work. I have to be carefull when I am going to work on bumblebee.

Samsagax commented 13 years ago

Should be a good idea to say something about Commit messages (as they are part of coding):

Should be a line with the idea involving the present change, a blank line and then some more in-depth explanation on the change (not a bible, but some hint to others) so we don't have to diff every file. Unless, of course the change is little and obvious.

A really bad commit message:

Deleted some stuff

A good one:

Deleted some power-management useless files

  - Files under pm/useless are all gone
  - Files under pm/maybeused are moved to pm/used
Lekensteyn commented 13 years ago

I second that, the commit message should be clear and useful. Consider http://stackoverflow.com/q/43598/427545

Keep the commit message within 72 characters, split over lines if necessary.

Fixed data loss bug

(optional additional description)

- Fixed a bug which could lead to data loss if it's night and the user 
  about to commit some changes (Closes: GH-123)

- Removed obsolete references to Prime-NG (GH-554: Clean Up)

Note the use of GH-<number>, it links to an issue. If it's Closes: GH-<number>, it also closes issue <number>. An extra newline should exist between every new line.

The PPA changelog needs some work too, not just "based on changes XXX", but:

PACKAGE (VERSION) UNRELEASED; urgency=low

  * New upstream release.
    - Message based on git changelog, that's why we expand it right?
    - Fix a bug which could lead to data loss (GH-123)
  * Main point (mind the two spaces in front of it)
    - some clarification (mind the four spaces in front of it)
    - we do not need to include all changes here, especially changes to whitespace

 -- User Name <name@example.com>  Thu, 04 Aug 2011 10:34:35 +0200
ArchangeGabriel commented 13 years ago

Ok, will try to do that from now.

Samsagax commented 13 years ago

Some further notice about coding: We must use the 'install' command instead of 'cp' + 'chmod'. Some distros even advice that usage for packaging. So we can set permissions and create folders only if needed: example: install bumblebee-config in /usr/bin owned by root with rw permission, read for the rest and execute by all:

install -D -m755 bumblebee-config /usr/bin/bumblebee-config