klendathu2k
commented
7 months ago Hi Irakli,
Yes and no. Yes, this is part of a general cleanup of Stv. Yes, if there is anything left it will not break things. No, there is not a zero risk of this breaking code. In particular the changes to StBFChain.
There were several places where the presence (or lack-thereof) of an Stv chain option altered the control flow. Many of them were logical OR's, which were easy to refactor...
if ( A || B || GetOption("Stv") ) { ... }
A few were slightly more complex. I likely got those right, but it would be good to have someone carefully review the changes to StBFChain.
Cheers, Jason
There were several instances in StBFChain where the
presence of the Stv option, the StvMaker, etc... was checked in logic.
I believe
that I have
On 2023-10-18 14:18, Irakli Chakaberia wrote:
@iraklic approved this pull request.
As I understand this is just a cleanup of Stv, and I assume this is full cleanup, I have not checked if there is Svt in any other option before the approval. But I also assume even if there is leftover it won't brake anything anyhow.
-- Reply to this email directly, view it on GitHub [1], or unsubscribe [2]. You are receiving this because you authored the thread.Message ID: @.***>
Links:
[1] https://github.com/star-bnl/star-sw/pull/607#pullrequestreview-1685815613 [2] https://github.com/notifications/unsubscribe-auth/ANL4LVDDLBX7DN3ZBJFWG3DYAAMPTAVCNFSM6AAAAAA6ELLFHGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMOBVHAYTKNRRGM
iraklic
Creating a draft PR. Additional Stv cleanup:
Removes IDL files. Removes chain options, and the configuration logic in StBFChain... (note: broken out over several commits as I refactored the slightly complicated control flow...)
Removed the StXTrakMaker. This depends on StvUtil. StvUtil was removed, but... our code still compiles. And I don't see any other xtrak dependencies... so draft PR should reveal whether it is a problem or not.