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

            swinbank John Swinbank created issue -
            swinbank John Swinbank made changes -
            Field Original Value New Value
            Epic Link DM-5395 [ 23202 ]
            swinbank John Swinbank made changes -
            Assignee Pim Schellart [ pschella ]
            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.
            pschella Pim Schellart [X] (Inactive) made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            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.
            pschella Pim Schellart [X] (Inactive) made changes -
            Reviewers Russell Owen [ rowen ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            pschella Pim Schellart [X] (Inactive) made changes -
            Link This issue duplicates DM-4008 [ DM-4008 ]
            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.
            rowen Russell Owen made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            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.
            pschella Pim Schellart [X] (Inactive) made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            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 .
            pschella Pim Schellart [X] (Inactive) made changes -
            Link This issue is triggering DM-5966 [ DM-5966 ]
            tjenness Tim Jenness made changes -
            Link This issue is triggered by RFC-100 [ RFC-100 ]
            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!
            pschella Pim Schellart [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ]
            Hide
            pschella Pim Schellart [X] (Inactive) added a comment -

            Done.

            Show
            pschella Pim Schellart [X] (Inactive) added a comment - Done.
            lauren Lauren MacArthur made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            lauren Lauren MacArthur made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            swinbank John Swinbank made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            swinbank John Swinbank made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            nlust Nate Lust made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            jbosch Jim Bosch made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            jbosch Jim Bosch made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            swinbank John Swinbank made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            swinbank John Swinbank made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            swinbank John Swinbank made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            swinbank John Swinbank made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            swinbank John Swinbank made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]
            swinbank John Swinbank made changes -
            Remote Link This issue links to "Page (Confluence)" [ 13633 ] This issue links to "Page (Confluence)" [ 13633 ]

              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