Skip to content

Conversation

@geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Jul 16, 2024

Extends the work on #15501 to fully include CPP.

@aschackmull please could you check I haven't left anything out.

@geoffw0 geoffw0 added the C++ label Jul 16, 2024
@geoffw0 geoffw0 requested a review from a team as a code owner July 16, 2024 09:06
Copy link
Contributor

@aschackmull aschackmull left a comment

Choose a reason for hiding this comment

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

LGTM. A few other languages that also have support for QL-defined csv rows use "SourceModelCsv", "SinkModelCsv", and "SummaryModelCsv" to get some provenance for those models as well, so you may want that as opposed to model = "". On the other hand, if you don't have too many QL-defined csv models, then the better option might just be to do a bit of follow-up work that converts them to yml and then deprecate the csv extension points, as that's really just legacy from before we had yml-based extensions.

@geoffw0
Copy link
Contributor Author

geoffw0 commented Jul 16, 2024

if you don't have too many QL-defined csv models, then the better option might just be to do a bit of follow-up work that converts them to yml and then deprecate the csv extension points, as that's really just legacy from before we had yml-based extensions.

I think that accurately describes the situation in CPP - MaD was only added recently and there are just a couple of QL-based, non-test CSV extension points. I'll create an issue to migrate them to .YML and deprecate the classes.

@geoffw0 geoffw0 merged commit 98319be into github:main Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants