View Issue Details

IDProjectCategoryView StatusLast Update
0024636Open CASCADEOCCT:Documentationpublic2014-05-05 13:38
Reporterkgv Assigned Toapn  
PrioritynormalSeverityintegration request 
Status closedResolutionfixed 
Target Version6.7.1Fixed in Version6.7.1 
Summary0024636: Coding Rules - define rules for development of Draw Harness commands
DescriptionExisting code base of Draw Harness commands significantly fluctuates in command line syntax, arguments processing, safety checks, output printing.

It is suggested to define rules / best practices for development of Draw Harness command to follow common style in future.
TagsNo tags attached.
Test case numberNot needed

Relationships

parent of 0024685 closedbugmaster Coding Rules - "Draw:Atoi" misprints in the document 

Activities

kgv

2014-02-17 23:38

developer   ~0027937

Dear abv,

please review the first version in CR24636 branch.

abv

2014-02-18 08:49

manager   ~0027939

No remarks, please proofread (and check generating documentation)

ysn

2014-02-20 19:52

developer   ~0027988

Reviewed, corrected grammar, style, formatting and logics. Removed redundancies.

abv

2014-02-20 20:10

manager   ~0027989

Yuri, thank you for the revision!

Please double check a few points I noticed:

- it seems that some words are lost in the text. For instance, at line 36 probably 'look' is missing before 'at the existing names of toolkits'. There are more examples in the text.

- if use of ... and other HTML tags is correctly handled by PDF generator

kgv

2014-02-20 21:53

developer   ~0027990

Dear Yuri,

in general it is bad idea to include changes unrelated to the issue description.
Such changes should be done within dedicated issue.

> until all major compliers on all supported platforms
Could you please fix misprint in addition, while you here (compliers -> compilers)?

> should be optional (avoided with appropriate preprocessor checks)
_avoided_ is inappropriate word for this message (originally used "escaped" makes sense here).

-If such iteration is used, make sure that the result of the algorithm does not depend on order.
+If such iteration is used, make sure that the result of the algorithm does not depend on the order if iterated items.

_if_ iterated -> _of_ iterated?

 ~~~~~{.cpp}
-Standard_Integer awidthofbox;  // this is bad
+Standard_Integer widthofbox;  // this is bad
 Standard_Integer width_of_box; // this is bad
-Standard_Integer aWidthOfBox;  // this is OK
+Standard_Integer WidthOfBox;  // this is OK
 ~~~~~

What a hell? Rule has been corrupted!
Please don't touch code samples.

-It is preferred to name public methods from upper case, while protected and private methods from low case.
+It is preferred to start names public methods from an upper case character and to start names of protected and private methods from a lower case character.

start names _OF_ public methods?

+The name of a local variable name should be distinguishable from the name of a function parameter, a class member variable and a global variable.
+

namename

+Avoid dummy names, such as I, j, k

Please fix pre-existed misprint here - I should be lowercase.

-In all sources try not to exceed 120 characters limit of line length.
+
+Try to stay within the limit of 120 characters per line in all sources .

redundant space before period.

-Single-line conditional operator (if, while, for etc.) can be written without brackets on the following line.
+
+Single-line conditional operators (*if, while, for, etc.) can be written without brackets on the following line.

broken formatting?

+
+This rule improves readability, allows detecting useless  multiple header inclusions and makes 3rd-party dependencies clearly visible.

+Following these rules is important for good comprehension of the comments. Moreover, this approach  allows automatically generating user-oriented documentation directly from the commented sources.
+

double space between words.

ysn

2014-02-21 11:52

developer   ~0027991

Thanks for the reviewing.

I should have rechecked my own changes.

One question:
" > should be optional (avoided with appropriate preprocessor checks)
_avoided_ is inappropriate word for this message (originally used "escaped" makes sense here). "

Meaning of "escape" (in computer science) = "To interrupt a command, exit a program, or change levels within a program by using a key, combination of keys, or key sequence."

So all optional "compiler-specific features" can be interrupted with "appropriate preprocessor checks". Is it what you wish to say?

kgv

2014-02-21 12:02

developer   ~0027992

Last edited: 2014-02-21 12:02

> Meaning of "escape" (in computer science) = "To interrupt a command, exit a program, or change levels within a program by using a key, combination of keys, or key sequence."

"Escaping" might has meaning "экранирование" in development (see "escape character" as most often use case).

The whole message might be better written as:
> should be optional (used only within appropriate preprocessor checks)

ysn

2014-02-21 16:26

developer   ~0027993

Remarks taken into account. Correct PDF generation checked.

kgv

2014-02-21 16:49

developer   ~0027994

Dear bugmaster,

please integrate the patch.
Please make sure that final commit message will not include triple copy of bug name.

Related Changesets

occt: master 49663e13

2014-02-27 14:39:32

kgv


Committer: apn Details Diff
0024636: Coding Rules - define rules for development of Draw Harness commands

.md file corrected grammar, style and logic. Removed redundancies.
Affected Issues
0024636
mod - dox/dev_guides/contribution/coding_rules.md Diff File

Issue History

Date Modified Username Field Change
2014-02-17 20:31 kgv New Issue
2014-02-17 20:31 kgv Assigned To => abv
2014-02-17 20:31 kgv Assigned To abv => kgv
2014-02-17 23:38 kgv Note Added: 0027937
2014-02-17 23:38 kgv Assigned To kgv => abv
2014-02-17 23:38 kgv Status new => resolved
2014-02-18 08:49 abv Note Added: 0027939
2014-02-18 08:49 abv Assigned To abv => ysn
2014-02-18 08:49 abv Status resolved => reviewed
2014-02-20 19:52 ysn Note Added: 0027988
2014-02-20 20:10 abv Note Added: 0027989
2014-02-20 21:53 kgv Note Added: 0027990
2014-02-21 11:52 ysn Note Added: 0027991
2014-02-21 12:02 kgv Note Added: 0027992
2014-02-21 12:02 kgv Note Edited: 0027992
2014-02-21 16:26 ysn Note Added: 0027993
2014-02-21 16:49 kgv Note Added: 0027994
2014-02-21 16:49 kgv Assigned To ysn => bugmaster
2014-02-25 16:15 apn Test case number => Not needed
2014-02-25 16:15 apn Status reviewed => tested
2014-03-03 13:25 apn Changeset attached => occt master 49663e13
2014-03-03 13:25 apn Assigned To bugmaster => apn
2014-03-03 13:25 apn Status tested => verified
2014-03-03 13:25 apn Resolution open => fixed
2014-03-04 19:35 kgv Relationship added related to 0024685
2014-03-05 14:28 kgv Relationship replaced parent of 0024685
2014-05-05 13:33 aiv Status verified => closed
2014-05-05 13:38 aiv Fixed in Version => 6.7.1