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.

Sign up for GitHub

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

Jump to bottom

refactor: Default to open extensions in branch of any_block_builder #616

Closed
wants to merge 1 commit into from
Closed

refactor: Default to open extensions in branch of any_block_builder #616

wants to merge 1 commit into from

Conversation

croyzor
Copy link
Contributor

@croyzor croyzor commented Oct 23, 2023

No description provided.

@croyzor croyzor requested a review from acl-cqc October 23, 2023 13:00
acl-cqc
acl-cqc reviewed Oct 23, 2023
View reviewed changes
Copy link
Contributor

@acl-cqc acl-cqc left a comment

Choose a reason for hiding this comment

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

If we made add_op_with_parent automatically use add_node_with_parent + open_extensions, then this change would have no effect, right.

#618 appears to do that? So if #618 works...???

@croyzor
Copy link
Contributor Author

croyzor commented Oct 23, 2023

If we made add_op_with_parent automatically use add_node_with_parent + open_extensions, then this change would have no effect, right.

Yes, but...

There are other callers of add_op_with_parent, so that doesn't quite work to fix this problem without breaking other things. Also add_node_with_parent, calls add_node which means that we can't delete it in #618, only generalise the add_op version

@croyzor croyzor mentioned this pull request Oct 23, 2023
Closed
@acl-cqc
Copy link
Contributor

acl-cqc commented Oct 31, 2023

Superceded by #622

@acl-cqc acl-cqc closed this Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acl-cqc acl-cqc acl-cqc left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@croyzor @acl-cqc