plan

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

plan

Barend Gehrels
Hi,

To summarize, the plan is on April 25:

1) reflect the folder structure to the one in the boost sandbox, so:
  - boost/ggl with headers
  - libs/ggl with tests, examples, documentation
 
2) change "geometry" to "ggl" in #include statements, so e.g. #include
<ggl/core/ring_type.hpp>

3) change "geometry" to "ggl" in namespaces

4) not indent namespaces

5) replace tabs with 4 spaces

6) in the test folder create the structure as in the headerfiles, so
.../algorithms, ../core, ../strategies, etc

7) rename fromwkt.hpp to read_wkt.hpp and aswkt.hpp to write_wkt.hpp,
and then also streamwkt.hpp to stream_wkt.hpp

8) move the stuff in multi which is not yet structured to the right
subfolders

All these cosmetic changes will be atomic and I will do most of them
"automatic", I will update the MS project files as well.

Regards, Barend



Reply | Threaded
Open this post in threaded view
|

plan

Mateusz Loskot
Administrator
Barend Gehrels wrote:
> Hi,
>
> To summarize, the plan is on April 25:
>
> 1) reflect the folder structure to the one in the boost sandbox, so:
>  - boost/ggl with headers
>  - libs/ggl with tests, examples, documentation

+1

> 2) change "geometry" to "ggl" in #include statements, so e.g. #include
> <ggl/core/ring_type.hpp>

+1

> 3) change "geometry" to "ggl" in namespaces

+1

> 4) not indent namespaces

+1

I've already fixed it in io/wkt (r591)

> 5) replace tabs with 4 spaces

+1

> 6) in the test folder create the structure as in the headerfiles, so
> .../algorithms, ../core, ../strategies, etc

+1

> 7) rename fromwkt.hpp to read_wkt.hpp and aswkt.hpp to write_wkt.hpp,
> and then also streamwkt.hpp to stream_wkt.hpp

+1

> 8) move the stuff in multi which is not yet structured to the right
> subfolders

+1

> All these cosmetic changes will be atomic and I will do most of them
> "automatic", I will update the MS project files as well.

OK. thanks!
I can update Jamfiles.

By the way, I've taken the liberty to apply one more cosmetic change:
I replaced the TOK aliase with tokenizer name in io/wkt:

typedef boost::tokenizer<boost::char_separator<char> > TOK;

reads now:

typedef boost::tokenizer<boost::char_separator<char> > tokenizer;

If you'd agree, I'd suggest to use upper-case for template parameters
only, otherwise it's hard to decipher what is what when reading through
code and requires to go back up to function/type prototype to find out.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

plan

Bruno Lalande
Hi,

> If you'd agree, I'd suggest to use upper-case for template parameters
> only, otherwise it's hard to decipher what is what when reading through
> code and requires to go back up to function/type prototype to find out.

Oh I wanted to talk about that, and had finally forgotten. I
personally think that UPPERCASE should only be used for macros and
constant literals, BTW I wonder if there's not a Boost guideline or
implicit rule prohibiting the use of UPPERCASE in any other context.
In Boost, typedefs are usually in lower_case like you did (exactly
like types, which is consistent) and template parameters are
UpperCase. That's why concept names are usually UpperCase as well in
the docs, since they end up as template parameters in the code. What
do you think about all that?

Bruno
Reply | Threaded
Open this post in threaded view
|

plan

Barend Gehrels
In reply to this post by Mateusz Loskot

>
> If you'd agree, I'd suggest to use upper-case for template parameters
> only, otherwise it's hard to decipher what is what when reading through
> code and requires to go back up to function/type prototype to find out.
+1 :-)
Barend



Reply | Threaded
Open this post in threaded view
|

plan

Barend Gehrels
In reply to this post by Bruno Lalande

> Oh I wanted to talk about that, and had finally forgotten. I
> personally think that UPPERCASE should only be used for macros and
> constant literals, BTW I wonder if there's not a Boost guideline or
> implicit rule prohibiting the use of UPPERCASE in any other context.
> In Boost, typedefs are usually in lower_case like you did (exactly
> like types, which is consistent) and template parameters are
> UpperCase. That's why concept names are usually UpperCase as well in
> the docs, since they end up as template parameters in the code. What
> do you think about all that?
>  
OK, these are indeed the guidelines.

    * Names (except as noted below) should be all lowercase, with words
      separated by underscores.
    * Template parameter names begin with an uppercase letter.

So we'll follow them.

Most template parameters are single char's (I think this is allowed...)
but where they are not (I sometimes used STRATEGY, SEG / SEC) they have
to be changed coming time.

Barend

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osgeo.org/pipermail/ggl/attachments/20090422/bd70c734/attachment.html
Reply | Threaded
Open this post in threaded view
|

plan

Barend Gehrels
In reply to this post by Mateusz Loskot

>
>> To summarize, the plan is on April 25:
>>
> I can update Jamfiles.
>
Thanks. Is everything checked in now? Then I'll start.
Barend




Reply | Threaded
Open this post in threaded view
|

plan

Mateusz Loskot
Administrator
Barend Gehrels wrote:
>>> To summarize, the plan is on April 25:
>>>
>> I can update Jamfiles.
>>
> Thanks. Is everything checked in now? Then I'll start.

Hi Barend,

I'm sorry I didn't let you know earlier.
I've been out of the office and disconnected from the Internet.
I've no pending checkins, so you can go on.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

plan

Barend Gehrels

> I'm sorry I didn't let you know earlier.
> I've been out of the office and disconnected from the Internet.
> I've no pending checkins, so you can go on.
>  
No problem


> 1) reflect the folder structure to the one in the boost sandbox, so:
>  - boost/ggl with headers
>  - libs/ggl with tests, examples, documentation
Done
>
> 2) change "geometry" to "ggl" in #include statements, so e.g. #include
> <ggl/core/ring_type.hpp>
Done (also renamed include guards from _GEOMETRY_ to GGL_, this was
automatic, not yet inserted subfolders)
>
> 3) change "geometry" to "ggl" in namespaces
Done
>
> 4) not indent namespaces
Done
>
> 5) replace tabs with 4 spaces
Done
>
> 6) in the test folder create the structure as in the headerfiles, so
> .../algorithms, ../core, ../strategies, etc
Done
>
> 7) rename fromwkt.hpp to read_wkt.hpp and aswkt.hpp to write_wkt.hpp,
> and then also streamwkt.hpp to stream_wkt.hpp
> 8) move the stuff in multi which is not yet structured to the right
> subfolders
Not yet done, will be done soon.

Everything is currently committed again. Note that there was one error
during renaming/copying, resulting in some svn-errors (such as copyflag
in properties, etc). Therefore the svn-history in multi* is lost, sorry
about that. I was able to keep the history in strategies etc (which is
much more interesting anyway).

Regards, Barend



Reply | Threaded
Open this post in threaded view
|

plan

Mateusz Loskot
Administrator
Barend Gehrels wrote:
>> 2) change "geometry" to "ggl" in #include statements, so e.g. #include
>> <ggl/core/ring_type.hpp>
>
> Done (also renamed include guards from _GEOMETRY_ to GGL_, this was
> automatic, not yet inserted subfolders)

I'm wondering if we shouldn't including using path relative to boost,
for example in cooridnate_type.hpp:

// boost
#include <boost/type_traits/remove_const.hpp>
// ggl
#include <boost/ggl/core/point_type.hpp>

>> 7) rename fromwkt.hpp to read_wkt.hpp and aswkt.hpp to write_wkt.hpp,
>> and then also streamwkt.hpp to stream_wkt.hpp
>> 8) move the stuff in multi which is not yet structured to the right
>> subfolders
>
> Not yet done, will be done soon.

> Everything is currently committed again. Note that there was one error
> during renaming/copying, resulting in some svn-errors (such as copyflag
> in properties, etc). Therefore the svn-history in multi* is lost, sorry
> about that. I was able to keep the history in strategies etc (which is
> much more interesting anyway).

I'm going to update tests accordingly.

Big thanks for the transition job!

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

plan

Barend Gehrels


> I'm wondering if we shouldn't including using path relative to boost,
> for example in cooridnate_type.hpp:
>
> // boost
> #include <boost/type_traits/remove_const.hpp>
> // ggl
> #include <boost/ggl/core/point_type.hpp>
>  
That is actually anticipating on the acceptance of boost, which is not
sure yet. It is allowed but I prefer to not yet do that.

By the way, I would omit the comments on include-order as it is not
common in Boost. We know the order and it is clear by folders and filenames.


>>> 7) rename fromwkt.hpp to read_wkt.hpp and aswkt.hpp to write_wkt.hpp,
>>> and then also streamwkt.hpp to stream_wkt.hpp
>>> 8) move the stuff in multi which is not yet structured to the right
>>> subfolders
>>>      
>> Not yet done, will be done soon.
>>    
Both done now. I also updated the Doxyfile and the documentation
(generation succeeds but didn't yet check the results)

> I'm going to update tests accordingly.
>
> Big thanks for the transition job!
>
>  
My pleasure. It is probably not yet perfect. I think most things are
handled. But we'll encounter things next days or weeks, they can be
solved then.

Regards, Barend



Reply | Threaded
Open this post in threaded view
|

plan

Mateusz Loskot
Administrator
Barend Gehrels wrote:

>> I'm wondering if we shouldn't including using path relative to boost,
>> for example in cooridnate_type.hpp:
>>
>> // boost
>> #include <boost/type_traits/remove_const.hpp>
>> // ggl
>> #include <boost/ggl/core/point_type.hpp>
>  
> That is actually anticipating on the acceptance of boost, which is not
> sure yet. It is allowed but I prefer to not yet do that.

OK.

> By the way, I would omit the comments on include-order as it is not
> common in Boost. We know the order and it is clear by folders and
> filenames.

Yes, you're right. I was adding some of these comments
before we decided on the conventions. I'll remove it.

>> I'm going to update tests accordingly.
>>
>> Big thanks for the transition job!
>>
>  
> My pleasure. It is probably not yet perfect. I think most things are
> handled. But we'll encounter things next days or weeks, they can be
> solved then.

OK. I'm also testing & updating where necessary over the weekend.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

plan

Mateusz Loskot
Administrator
In reply to this post by Barend Gehrels
Barend Gehrels wrote:
>> 2) change "geometry" to "ggl" in #include statements, so e.g. #include
>> <ggl/core/ring_type.hpp>
>
> Done (also renamed include guards from _GEOMETRY_ to GGL_, this was
> automatic, not yet inserted subfolders)

FYI, I've started to complete subfolders in the guards

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
--
Mateusz Loskot
http://mateusz.loskot.net
Reply | Threaded
Open this post in threaded view
|

plan

Mateusz Loskot
Administrator
In reply to this post by Mateusz Loskot
Mateusz Loskot wrote:
> OK. I'm also testing & updating where necessary over the weekend.

I've just submitted big update to tests following recent
restructuring work that Barend made.

There are a few tests in trunk/libs/ggl/test/point_concept
which need some work, because of errors related to tags dispatching.
I'll check it later.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
Charter Member of OSGeo, http://osgeo.org
--
Mateusz Loskot
http://mateusz.loskot.net