Remove dktk and bbmri feature flags#283
Conversation
Previously we had samply/focus:main-dktk and samply/focus:main-bbmri, now we have samply/focus:main which can do both.
enola-dkfz
left a comment
There was a problem hiding this comment.
Get a green light from @lablans, features are his babies
26fd99c to
8ca3fd5
Compare
Threated
left a comment
There was a problem hiding this comment.
So previously this was just decided on the feature flag right?
So unknown projects that talked to a dktk focus would have gotten dktk semantics and did not get rejected?
So this, as is, is definitely a breaking change since there are a lot more "projects" that talk to the dktk focus like exliquid and dktk_supervisors (test lens). But I guess adding these is just a matter of adding enum variants and fixing compile errors.
The enum is only parsed if it is an AST query and many projects (e.g. exliquid) don't send AST. Maybe there are other projects than DKTK that want DKTK's AST->CQL semantics, we could add them to the enum. Another breaking change is that some projects might not send the project metadata field at all right now. Would need to add |
8b9f81e to
9ca29d0
Compare
Threated
left a comment
There was a problem hiding this comment.
r=me for the code. But we should figure out which projects we might need to add before merging this
Threated
left a comment
There was a problem hiding this comment.
Totally fine with merging this to develop but we need to check that all projects using AST have the project metadata configured correctly.
projectmetadata fieldsamply/focus:main(previously we hadsamply/focus:main-dktk,samply/focus:main-bbmri)