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

Remove use of Boost smart pointers throughout the Science Pipelines

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Story Points:
      10
    • Sprint:
      DRP X16-3
    • Team:
      Data Release Production

      Description

      Replace all use of Boost smart pointers through the stack with their standard library equivalents.

      This will require an RFC.

        Attachments

          Issue Links

            Activity

            Hide
            swinbank John Swinbank added a comment -

            Note that there has already been an RFC on this topic: RFC-100. There's also an implementation ticket: DM-4008. We should probably close this as a duplicate; let's confirm with Simon Krughoff first.

            Show
            swinbank John Swinbank added a comment - Note that there has already been an RFC on this topic: RFC-100 . There's also an implementation ticket: DM-4008 . We should probably close this as a duplicate; let's confirm with Simon Krughoff first.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Full stack boost::scoped_ptr to std::unique_ptr conversion complete on u/pschella/DM-5879 branch. Changes in afw, daf_base, daf_persistance, meas_modelfit, pex_logging, pex_policy and shapelet packages. A ci run has completed successfully.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Full stack boost::scoped_ptr to std::unique_ptr conversion complete on u/pschella/ DM-5879 branch. Changes in afw, daf_base, daf_persistance, meas_modelfit, pex_logging, pex_policy and shapelet packages. A ci run has completed successfully.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            This ticket is a duplicate of DM-4008 (or the other way around) which Russell Owen originally filed, therefore requesting him to review.
            A ci build of the ticket branch has just completed successfully.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - This ticket is a duplicate of DM-4008 (or the other way around) which Russell Owen originally filed, therefore requesting him to review. A ci build of the ticket branch has just completed successfully.
            Hide
            rowen Russell Owen added a comment -

            Overall this looks great and I really appreciate your doing this.

            However, a few minor issues:

            • Your automatic replacement resulted in <memory> being imported twice in some cases. Not an error, but worth fixing.
            • We have a standard order for imports: first standard libraries (using "<...>"), then 3rd party packages (using "..." instead of <...>, but not worth fixing), then LSST code. Each group is supposed to be separated by a blank line; it's not worth fixing all instances where they are not, but if you stumble across these while regrouping <memory> I'd appreciate it if you'd fix those.
            Show
            rowen Russell Owen added a comment - Overall this looks great and I really appreciate your doing this. However, a few minor issues: Your automatic replacement resulted in <memory> being imported twice in some cases. Not an error, but worth fixing. We have a standard order for imports: first standard libraries (using "<...>"), then 3rd party packages (using "..." instead of <...>, but not worth fixing), then LSST code. Each group is supposed to be separated by a blank line; it's not worth fixing all instances where they are not, but if you stumble across these while regrouping <memory> I'd appreciate it if you'd fix those.
            Hide
            rowen Russell Owen added a comment -

            I only looked at the diffs (so if there are other places that changes are needed I will have missed those). The rest of my comments are above.

            Show
            rowen Russell Owen added a comment - I only looked at the diffs (so if there are other places that changes are needed I will have missed those). The rest of my comments are above.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Ah yes, I just noticed that too. Will fix. The second point is harder to automate though. Would be really nice if we could just use clang-format to enforce style and have it handle these things. Perhaps I should RFC/D that sometime. For now will do as you suggested.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Ah yes, I just noticed that too. Will fix. The second point is harder to automate though. Would be really nice if we could just use clang-format to enforce style and have it handle these things. Perhaps I should RFC/D that sometime. For now will do as you suggested.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Ok, includes have been fixed now.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Ok, includes have been fixed now.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Merge complete.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Merge complete.
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Note that ndarray still uses boost smart pointers. Have left this out of the ticket as suggested by Jim Bosch.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Note that ndarray still uses boost smart pointers. Have left this out of the ticket as suggested by Jim Bosch .
            Hide
            swinbank John Swinbank added a comment -

            Pim Schellart [X] – when you get a moment, could you please add a (brief!) mention of this to the "build & code improvements" section of the release notes? Thanks!

            Show
            swinbank John Swinbank added a comment - Pim Schellart [X] – when you get a moment, could you please add a (brief!) mention of this to the "build & code improvements" section of the release notes ? Thanks!
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Done.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Done.

              People

              • Assignee:
                pschella Pim Schellart [X] (Inactive)
                Reporter:
                swinbank John Swinbank
                Reviewers:
                Russell Owen
                Watchers:
                John Swinbank, Pim Schellart [X] (Inactive), Russell Owen
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel