DM-33453: Update pipeline subdirectories in response to RFC-775. #150
Conversation
There was a problem hiding this comment.
I think something's gone wrong with the move, because there are now both old-style and new-style directories: https://github.com/lsst/ap_pipe/tree/ad29150c5e43bd8d75438a7a013dac95e3120851/pipelines.
Please update pipelines/README.md; its example uses the old-style directory name. (I've checked with GitHub search that this seems to be the only dangling reference, but GH doesn't let you search branches so you might want to double-check.)
I'm also a bit concerned about the wording of the commit message -- it sounds as if we were arbitrarily following the DRP group's lead, rather than implementing something we all agreed to (RFC-775). Maybe nobody will ever read it, but it still feels like a misrepresentation.
There was a problem hiding this comment.
I'm not sure what the ../DECam/ApPipe.yaml line is supposed to be?
(Also, GitHub seems to be doing something weird with this file. I tried to add a comment to the added line and it said "Line must be part of the diff and Diff hunk can't be blank".)
There was a problem hiding this comment.
They're supposed to be symbolic links for backwards compatibility. I discussed it a bit with Ian and Eric, though there was no end date for when we'll get rid of the sym links.
There was a problem hiding this comment.
Ok, that makes sense. However, it's a bit problematic since we do need to get rid of those directories (that was the whole point!), and we don't have a way to add a deprecation warning to symlinks.
I think a hard break is the lesser evil in this case, but if not, please at least file a removal ticket and have one of the upcoming releases block on it as if we were following the deprecation policy.
EDIT: accidentally said the exact opposite of what would ensure this gets cleaned up eventually...
There was a problem hiding this comment.
Actually, one other request: could the symlinks be split off into their own commit? I think it might help git/GitHub be less confused about the net diff, as well as providing a natural division of the changes (essential vs. non-essential).
erinleighh
force-pushed
the
tickets/DM-33453
branch
2 times, most recently
from
76c6a13
to
c6b8e04
Compare
erinleighh
changed the title
| @@ -10,6 +10,6 @@ For example, to visualize the `apPipe` subset from the [LSSTCam-imSim ApPipe pip | |||
|
|
|||
There was a problem hiding this comment.
Oops, found a problem -- the link on line 9 needs to be updated as well.
erinleighh
force-pushed
the
tickets/DM-33453
branch
from
c6b8e04
to
146d1b8
Compare
Changed
HyperSuprimeCamtoHSC,DarkEnergyCameratoDECam, andLSSTCamImSimtoLSSTCam-imSimin response to RFC-775.