Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update documentation to be coherent with the style guide #4771

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 35 additions & 143 deletions doc/tutorials/content/writing_new_classes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ Setting up the structure

If you're not familiar with the PCL file structure already, please go ahead
and read the `PCL C++ Programming Style Guide
<http://www.pointclouds.org/documentation/advanced/pcl_style_guide.php>`_ to
<https://pcl.readthedocs.io/projects/advanced/en/latest/pcl_style_guide.html>`_ to
familiarize yourself with the concepts.

There're two different ways we could set up the structure: i) set up the code
Expand Down Expand Up @@ -193,8 +193,7 @@ skeleton:
.. code-block:: cpp
:linenos:

#ifndef PCL_FILTERS_BILATERAL_H_
#define PCL_FILTERS_BILATERAL_H_
#pragma once

#include <pcl/filters/filter.h>

Expand All @@ -206,8 +205,6 @@ skeleton:
};
}

#endif // PCL_FILTERS_BILATERAL_H_

bilateral.hpp
=============

Expand All @@ -217,12 +214,9 @@ While we're at it, let's set up two skeleton *bilateral.hpp* and
.. code-block:: cpp
:linenos:

#ifndef PCL_FILTERS_BILATERAL_IMPL_H_
#define PCL_FILTERS_BILATERAL_IMPL_H_
#pragma once

#include <pcl/filters/bilateral.h>

#endif // PCL_FILTERS_BILATERAL_IMPL_H_

This should be straightforward. We haven't declared any methods for
`BilateralFilter` yet, therefore there is no implementation.
Expand Down Expand Up @@ -511,8 +505,7 @@ header file becomes:
.. code-block:: cpp
:linenos:

#ifndef PCL_FILTERS_BILATERAL_H_
#define PCL_FILTERS_BILATERAL_H_
#pragma once

#include <pcl/filters/filter.h>
#include <pcl/kdtree/kdtree.h>
Expand Down Expand Up @@ -584,8 +577,6 @@ header file becomes:
};
}

#endif // PCL_FILTERS_BILATERAL_H_

bilateral.hpp
=============

Expand Down Expand Up @@ -651,17 +642,14 @@ entry for the class:
.. code-block:: cpp
:linenos:

#ifndef PCL_FILTERS_BILATERAL_IMPL_H_
#define PCL_FILTERS_BILATERAL_IMPL_H_
#pragma once

#include <pcl/filters/bilateral.h>

...

#define PCL_INSTANTIATE_BilateralFilter(T) template class PCL_EXPORTS pcl::BilateralFilter<T>;

#endif // PCL_FILTERS_BILATERAL_IMPL_H_

One additional thing that we can do is error checking on:

* whether the two `sigma_s_` and `sigma_r_` parameters have been given;
Expand Down Expand Up @@ -709,8 +697,7 @@ The implementation file header thus becomes:
.. code-block:: cpp
:linenos:

#ifndef PCL_FILTERS_BILATERAL_IMPL_H_
#define PCL_FILTERS_BILATERAL_IMPL_H_
#pragma once

#include <pcl/filters/bilateral.h>
#include <pcl/kdtree/kdtree_flann.h>
Expand Down Expand Up @@ -770,8 +757,6 @@ The implementation file header thus becomes:

#define PCL_INSTANTIATE_BilateralFilter(T) template class PCL_EXPORTS pcl::BilateralFilter<T>;

#endif // PCL_FILTERS_BILATERAL_IMPL_H_


Taking advantage of other PCL concepts
--------------------------------------
Expand Down Expand Up @@ -824,8 +809,7 @@ The implementation file header thus becomes:
.. code-block:: cpp
:linenos:

#ifndef PCL_FILTERS_BILATERAL_IMPL_H_
#define PCL_FILTERS_BILATERAL_IMPL_H_
#pragma once

#include <pcl/filters/bilateral.h>
#include <pcl/kdtree/kdtree_flann.h>
Expand Down Expand Up @@ -885,8 +869,6 @@ The implementation file header thus becomes:

#define PCL_INSTANTIATE_BilateralFilter(T) template class PCL_EXPORTS pcl::BilateralFilter<T>;

#endif // PCL_FILTERS_BILATERAL_IMPL_H_

To make :pcl:`indices_<pcl::PCLBase::indices_>` work without typing the full
construct, we need to add a new line to *bilateral.h* that specifies the class
where `indices_` is declared:
Expand Down Expand Up @@ -918,42 +900,14 @@ file, as follows:
.. code-block:: cpp
:linenos:

/*
* Software License Agreement (BSD License)
*
* Point Cloud Library (PCL) - www.pointclouds.org
* Copyright (c) 2010-2011, Willow Garage, Inc.
*
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution.
* * Neither the name of Willow Garage, Inc. nor the names of its
* contributors may be used to endorse or promote products derived
* from this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
*/
/*
* SPDX-License-Identifier: BSD-3-Clause
*
* Point Cloud Library (PCL) - www.pointclouds.org
* Copyright (c) 2014-, Open Perception Inc.
*
* All rights reserved
*/

An additional like can be inserted if additional copyright is needed (or the
original copyright can be changed):
Expand Down Expand Up @@ -983,45 +937,16 @@ class look like:
.. code-block:: cpp
:linenos:

/*
* Software License Agreement (BSD License)
*
* Point Cloud Library (PCL) - www.pointclouds.org
* Copyright (c) 2010-2011, Willow Garage, Inc.
*
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution.
* * Neither the name of Willow Garage, Inc. nor the names of its
* contributors may be used to endorse or promote products derived
* from this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
*/

#ifndef PCL_FILTERS_BILATERAL_H_
#define PCL_FILTERS_BILATERAL_H_
/*
* SPDX-License-Identifier: BSD-3-Clause
*
* Point Cloud Library (PCL) - www.pointclouds.org
* Copyright (c) 2014-, Open Perception Inc.
*
* All rights reserved
*/

#pragma once

#include <pcl/filters/filter.h>
#include <pcl/kdtree/kdtree.h>
Expand Down Expand Up @@ -1131,52 +1056,21 @@ class look like:
};
}

#endif // PCL_FILTERS_BILATERAL_H_

And the *bilateral.hpp* likes:

.. code-block:: cpp
:linenos:

/*
* Software License Agreement (BSD License)
*
* Point Cloud Library (PCL) - www.pointclouds.org
* Copyright (c) 2010-2011, Willow Garage, Inc.
*
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution.
* * Neither the name of Willow Garage, Inc. nor the names of its
* contributors may be used to endorse or promote products derived
* from this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
*/

#ifndef PCL_FILTERS_BILATERAL_IMPL_H_
#define PCL_FILTERS_BILATERAL_IMPL_H_
/*
* SPDX-License-Identifier: BSD-3-Clause
*
* Point Cloud Library (PCL) - www.pointclouds.org
* Copyright (c) 2014-, Open Perception Inc.
*
* All rights reserved
*/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could use this PR to update also the LICENSE blurb to use the new one when writing a class. What do you think @kunaltyagi ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blurb in this rst file? Sure. But not in actual code files

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, speaking about the doc, not actual code!

Copy link
Contributor Author

@tin1254 tin1254 May 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one? Maybe we should also add this to the style guide in a new PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be considered a style guide? Isn't that more for code? We do have a blurb in contributing.md for reference

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would consider it as style guide (Google Style Guide also include in their guide)

But maybe updating this file is enough for new contributors, as anyone wanted to implement a new class will definitely look at this rst

#pragma once

#include <pcl/filters/bilateral.h>
#include <pcl/kdtree/kdtree_flann.h>
Expand Down Expand Up @@ -1249,8 +1143,6 @@ And the *bilateral.hpp* likes:

#define PCL_INSTANTIATE_BilateralFilter(T) template class PCL_EXPORTS pcl::BilateralFilter<T>;

#endif // PCL_FILTERS_BILATERAL_IMPL_H_


Testing the new class
---------------------
Expand Down