Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: move SideBitFlags into Side #4768

Merged
merged 21 commits into from
Apr 6, 2022

Conversation

pollend
Copy link
Member

@pollend pollend commented Jun 14, 2021

Deprecate most of SideBitFlag and offer direct replacements in Side, basically merging the two classes.

Respective replacements are documented in the deprecation docstring.

This prepares for eventually removing SideBitFlag.

@github-actions github-actions bot added the Type: Chore Request for or implementation of maintenance changes label Jun 14, 2021
@pollend pollend marked this pull request as ready for review June 27, 2021 04:18
@pollend pollend added the Type: Refactoring Request for or implementation of pure and automatic refactorings, e.g. renaming, to improve clarity label Jun 27, 2021
@skaldarnar
Copy link
Member

The side bit flag is used in many places. Do you plan to migrate those in follow-up PRs, and if so, could you please create them for the Omega line-up?

I think merging SideBitFlag into Side itself does make it more accessible (and discoverable), but I'd like to round this up rather soon after starting the deprecation process 🙃

@pollend
Copy link
Member Author

pollend commented Aug 28, 2021

@skaldarnar it doesn't need to happen immediately the old interface is still there.

@skaldarnar skaldarnar changed the title chore: move SideBitFlags into side chore: move SideBitFlags into Side Aug 28, 2021
@skaldarnar
Copy link
Member

it doesn't need to happen immediately the old interface is still there.

I know, and I think the benefit here is that we are not blocked by this migration even while it's happening, and that we are also not forced to do everything at once.

However, with just this PR and no follow ups, we just created a second option where there was only one before, with no examples on how to use it or to motivate why the "new" implementation is better than the old one.
I'm concerned that this will become just another part of the code base that's in a weird shape because we started one thing we never finish.

Therefore, I'd like to repeat my request to at least start making the according PRs such that we can get this finished (i.e., remove SideBitFlags) within the next weeks.

skaldarnar
skaldarnar previously approved these changes Apr 6, 2022
skaldarnar
skaldarnar previously approved these changes Apr 6, 2022
@jdrueckert jdrueckert merged commit b41012d into develop Apr 6, 2022
@jdrueckert jdrueckert deleted the chore/move-SideBitFlags-into-Side branch April 6, 2022 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Chore Request for or implementation of maintenance changes Type: Refactoring Request for or implementation of pure and automatic refactorings, e.g. renaming, to improve clarity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants