Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-4639

modernize afw code and reduce doxygen errors

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw

      Description

      I would like to make some simple modernizations afw code and reduce doxygen warnings as much as practical. The modernizations I had in mind were:

      • Move doc strings from .cc files to .h files and standardize their format
      • Use namespace lsst { namespace afw { ... in .cc files to make the code easier to read
      • Eliminate all <Class>::Ptr and <Class>::ConstPtr typedefs (replacing with std::shared_ptr<Class>) and std::shared_ptr<const Class>).
      • Make sure .py files import the appropriate packages from future and (where practical) pass the flake8 linter
      • Run clang-format on the code.

      Regarding doxygen warnings: I think moving the documentation to .h files will help in many cases. Some warnings may be impractical to fix, such as complaining about not documenting "cls" for python class methods.

        Attachments

          Issue Links

            Activity

            Hide
            krzys Krzysztof Findeisen added a comment -

            Jim Bosch is there any documentation for the RFC, in case I need to defend the change myself?

            Show
            krzys Krzysztof Findeisen added a comment - Jim Bosch is there any documentation for the RFC, in case I need to defend the change myself?
            Hide
            jbosch Jim Bosch added a comment -

            The best I can find is an old trac ticket for removing all the Ptr and ConstPtr typedefs:

            https://dev.lsstcorp.org/trac/ticket/2486

            Maybe it wasn't an RFC. Maybe it preceded the RFC process. But I do remember it being Decided when we switched to the PTR macros.

            Show
            jbosch Jim Bosch added a comment - The best I can find is an old trac ticket for removing all the Ptr and ConstPtr typedefs: https://dev.lsstcorp.org/trac/ticket/2486 Maybe it wasn't an RFC. Maybe it preceded the RFC process. But I do remember it being Decided when we switched to the PTR macros.
            Hide
            swinbank John Swinbank added a comment -

            The decision to remove (Const)Ptr long predates the introduction of the RFC process. At least, when I arrived, "everybody knew" that they ought to be removed, and I predate the introduction of the RFC process.

            Show
            swinbank John Swinbank added a comment - The decision to remove (Const)Ptr long predates the introduction of the RFC process. At least, when I arrived, "everybody knew" that they ought to be removed, and I predate the introduction of the RFC process.
            Hide
            jbosch Jim Bosch added a comment -

            My review of comments and SWIG preprocessor stuff is complete. Comments on the PR (on just the Doxygen/Comments commit).

            Show
            jbosch Jim Bosch added a comment - My review of comments and SWIG preprocessor stuff is complete. Comments on the PR (on just the Doxygen/Comments commit).
            Hide
            krzys Krzysztof Findeisen added a comment -

            Merged with requested changes. Thank you, everyone!

            Show
            krzys Krzysztof Findeisen added a comment - Merged with requested changes. Thank you, everyone!

              People

              • Assignee:
                krzys Krzysztof Findeisen
                Reporter:
                rowen Russell Owen
                Reviewers:
                Pim Schellart [X] (Inactive), Russell Owen, Tim Jenness
                Watchers:
                Jim Bosch, John Swinbank, Krzysztof Findeisen, Pim Schellart [X] (Inactive), Russell Owen, Simon Krughoff, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel