MantisBT - Open CASCADE
View Issue Details
0028441Open CASCADE[OCCT] OCCT:Codingpublic2017-02-05 13:432017-09-29 16:24
kgv 
bugmaster 
lowintegration request 
closedfixed 
[OCCT] 6.5.4 
[OCCT] 7.2.0[OCCT] 7.2.0 
Not needed
0028441: Coding Rules - move out nested Image_PixMap::ImgFormat enumeration to dedicated enum Image_Format
Image_PixMap::ImgFormat enumeration is defined as enumeration nested into class Image_PixMap. This is not recommended by OCCT coding rules and makes extra problems in some contexts (e.g. automated wrapping into another language).
N/A
No tags attached.
related to 0023272closed kgv Image comparison algorithm 
Issue History
2017-02-05 13:43kgvNew Issue
2017-02-05 13:43kgvAssigned To => kgv
2017-02-05 13:48gitNote Added: 0063448
2017-02-05 13:48kgvNote Added: 0063449
2017-02-05 13:48kgvAssigned Tokgv => abv
2017-02-05 13:48kgvPrioritynormal => low
2017-02-05 13:48kgvSeverityminor => integration request
2017-02-05 13:48kgvStatusnew => resolved
2017-02-05 13:50kgvProduct Version7.1.0 => 6.5.4
2017-02-05 13:50kgvRelationship addedrelated to 0023272
2017-02-09 14:19abvNote Added: 0063731
2017-02-09 14:19abvAssigned Toabv => kgv
2017-02-09 14:19abvStatusresolved => assigned
2017-02-09 14:23kgvNote Added: 0063732
2017-02-14 12:16gitNote Added: 0063790
2017-02-14 12:17gitNote Added: 0063791
2017-02-14 12:18kgvNote Added: 0063792
2017-02-14 12:18kgvAssigned Tokgv => abv
2017-02-14 12:18kgvStatusassigned => resolved
2017-02-24 22:43abvNote Added: 0063990
2017-02-24 22:43abvAssigned Toabv => bugmaster
2017-02-24 22:43abvStatusresolved => reviewed
2017-02-27 16:35gitNote Added: 0064011
2017-02-27 16:35apvNote Added: 0064012
2017-02-27 16:35apvAssigned Tobugmaster => apv
2017-02-28 14:17apvTest case number => Not needed
2017-02-28 16:26apvNote Added: 0064033
2017-02-28 16:26apvAssigned Toapv => kgv
2017-02-28 16:26apvStatusreviewed => assigned
2017-02-28 16:27apvNote Added: 0064034
2017-02-28 18:21gitNote Added: 0064039
2017-02-28 18:21kgvNote Added: 0064040
2017-02-28 18:21kgvAssigned Tokgv => bugmaster
2017-02-28 18:21kgvStatusassigned => resolved
2017-02-28 18:21kgvStatusresolved => reviewed
2017-02-28 18:50apvAssigned Tobugmaster => apv
2017-03-01 14:36apvNote Added: 0064055
2017-03-01 14:36apvAssigned Toapv => bugmaster
2017-03-01 14:36apvStatusreviewed => tested
2017-03-03 16:25bugmasterChangeset attached => occt master dc858f4c
2017-03-03 16:25bugmasterStatustested => verified
2017-03-03 16:25bugmasterResolutionopen => fixed
2017-03-20 14:33gitNote Added: 0064466
2017-03-20 14:33gitNote Added: 0064469
2017-03-20 14:35gitNote Added: 0064487
2017-09-29 16:20aivFixed in Version => 7.2.0
2017-09-29 16:24aivStatusverified => closed

Notes
(0063448)
git   
2017-02-05 13:48   
Branch CR28441 has been created by kgv.

SHA-1: ca2a647e897d020d1092020c1697b0f1a2fa5d92


Detailed log of new commits:

Author: kgv
Date: Sun Feb 5 13:47:27 2017 +0300

    0028441: Coding Rules - move out nested Image_PixMap::ImgFormat enumeration to dedicated enum Image_Format
(0063449)
kgv   
2017-02-05 13:48   
Patch is ready for review.
(0063731)
abv   
2017-02-09 14:19   
Please describe this change in commit message and upgrade guide. I think that it can be also instrumental to add corresponding rename record in section [rename] of adm/upgrade.dat, like this (if relevant):

Image_PixMap::Img Image_Format_

I noticed that you moved RTTI declaration for the class Image_AlienPixMap to its private section -- is this intentional? If yes, why?

Deprecation message

"This enumeration is deprecated, Image_Format should be called instead"

can be probably made more clear, e.g.:

"This member is deprecated, use Image_Format enumeration instead"
(0063732)
kgv   
2017-02-09 14:23   
> I noticed that you moved RTTI declaration for the class Image_AlienPixMap
> to its private section -- is this intentional? If yes, why?
DEFINE_STANDARD_RTTIEXT() starts with "public:", so that type definition become more compact and near to class definition itself.
(0063790)
git   
2017-02-14 12:16   
Branch CR28441 has been updated by kgv.

SHA-1: 34a95ef4e941aa3c2294d02ced40f4997a970665


Detailed log of new commits:

Author: kgv
Date: Tue Feb 14 12:16:12 2017 +0300

    Enumeration Image_PixMap::ImgFormat, previously declared as nested
    enumeration within class *Image_PixMap*,
    has been moved to global namespace as Image_Format following OCCT coding rules.
    
    The enumeration values have suffix Image_Format_ and preserve
    previous name scheme for easy renaming of old values.
    E.g. Image_PixMap::ImgGray become Image_Format_Gray.
    Old definitions are preserved as depreacated aliases to the new ones.

(0063791)
git   
2017-02-14 12:17   
Branch CR28441_1 has been created by kgv.

SHA-1: e3c9e0f081bde89c02b3506545d213219a9dcd5c


Detailed log of new commits:

Author: kgv
Date: Sun Feb 5 13:47:27 2017 +0300

    0028441: Coding Rules - move out nested Image_PixMap::ImgFormat enumeration to dedicated enum Image_Format
    
    Enumeration Image_PixMap::ImgFormat, previously declared as nested
    enumeration within class *Image_PixMap*,
    has been moved to global namespace as Image_Format following OCCT coding rules.
    
    The enumeration values have suffix Image_Format_ and preserve
    previous name scheme for easy renaming of old values.
    E.g. Image_PixMap::ImgGray become Image_Format_Gray.
    Old definitions are preserved as depreacated aliases to the new ones.
(0063792)
kgv   
2017-02-14 12:18   
Updated patch has been pushed with applied remarks.
(0063990)
abv   
2017-02-24 22:43   
No remarks, please test
(0064011)
git   
2017-02-27 16:35   
Branch CR28441_1 has been updated forcibly by apv.

SHA-1: 8fea322af71a9fc14f0b8173c8f63922837f6619
(0064012)
apv   
2017-02-27 16:35   
Branch CR28441_1 has been rebased on the current master
(0064033)
apv   
2017-02-28 16:26   
Dear BugMaster,

Branch CR28441_1 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
SHA-1: 8fea322af71a9fc14f0b8173c8f63922837f6619

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 15 (0 on master)
   MasOS: 0 (0 on master)
products component:
   Linux: 64
   Windows: 0
   MacOS: 1182
New warnings have been detected during OCCT component building on Windows:
http://jenkins-test-07.nnov.opencascade.com/view/CR28441_1-master/job/CR28441_1-master-OCCT-Windows-64-VC10-opt-compile/1/warnings34Result/ [^]

Regressions/Differences:
Not detected

Testing cases:
Not needed

Testing on Linux:
Total MEMORY difference: 93011325 / 93130577 [-0.13%]
Total CPU difference: 19965.06000000016 / 19801.410000000218 [+0.83%]

Testing on Windows:
Total MEMORY difference: 57832414 / 57829936 [+0.00%]
Total CPU difference: 18628.563013098457 / 18793.846072598604 [-0.88%]
(0064034)
apv   
2017-02-28 16:27   
Dear Kirill,

Branch CR28441_1 has been rejected due to:
- additional warnings
(0064039)
git   
2017-02-28 18:21   
Branch CR28441_2 has been created by kgv.

SHA-1: 472338e8430267be34abefc0ccfe4de276b5883d


Detailed log of new commits:

Author: kgv
Date: Sun Feb 5 13:47:27 2017 +0300

    0028441: Coding Rules - move out nested Image_PixMap::ImgFormat enumeration to dedicated enum Image_Format
    
    Enumeration Image_PixMap::ImgFormat, previously declared as nested
    enumeration within class *Image_PixMap*,
    has been moved to global namespace as Image_Format following OCCT coding rules.
    
    The enumeration values have suffix Image_Format_ and preserve
    previous name scheme for easy renaming of old values.
    E.g. Image_PixMap::ImgGray become Image_Format_Gray.
    Old definitions are preserved as depreacated aliases to the new ones.
(0064040)
kgv   
2017-02-28 18:21   
Please check compilation of updated patch in branch CR28441_2.
Regression testing is not needed.
(0064055)
apv   
2017-03-01 14:36   
Dear BugMaster,

Branch CR28441_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms.
SHA-1: 472338e8430267be34abefc0ccfe4de276b5883d

Number of compiler warnings:
occt component:
   Linux: 0 (0 on master)
   Windows: 0 (0 on master)
   MasOS: 0 (0 on master)
products component:
   Linux: 64
   Windows: 0
   MacOS: 1193
(0064466)
git   
2017-03-20 14:33   
Branch CR28441_2 has been deleted by inv.

SHA-1: 472338e8430267be34abefc0ccfe4de276b5883d
(0064469)
git   
2017-03-20 14:33   
Branch CR28441_1 has been deleted by inv.

SHA-1: 8fea322af71a9fc14f0b8173c8f63922837f6619
(0064487)
git   
2017-03-20 14:35   
Branch CR28441 has been deleted by inv.

SHA-1: 34a95ef4e941aa3c2294d02ced40f4997a970665