MantisBT
Mantis Bug Tracker Workflow

View Issue Details Jump to Notes ] Related Changesets ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0024636Open CASCADE[OCCT] OCCT:Documentationpublic2014-02-17 20:312014-05-05 13:38
Reporterkgv 
Assigned Toapn 
PrioritynormalSeverityintegration request 
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version[OCCT] 6.7.1Fixed in Version[OCCT] 6.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
Attached Files

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

-  Notes
(0027937)
kgv (developer)
2014-02-17 23:38

Dear abv,

please review the first version in CR24636 branch.
(0027939)
abv (manager)
2014-02-18 08:49

No remarks, please proofread (and check generating documentation)
(0027988)
ysn (developer)
2014-02-20 19:52

Reviewed, corrected grammar, style, formatting and logics. Removed redundancies.
(0027989)
abv (manager)
2014-02-20 20:10

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
(0027990)
kgv (developer)
2014-02-20 21:53

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.
(0027991)
ysn (developer)
2014-02-21 11:52

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?
(0027992)
kgv (developer)
2014-02-21 12:02
edited on: 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)

(0027993)
ysn (developer)
2014-02-21 16:26

Remarks taken into account. Correct PDF generation checked.
(0027994)
kgv (developer)
2014-02-21 16:49

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
Timestamp: 2014-02-27 14:39:32
Author: 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.
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 View Revisions
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


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker