d7_comment migration should have dependency on d7_comment_entity_display, same for d7_custom_block + block_content_entity_display

Not sure adding optional dependencies is a good idea or not. I'll leave it to other maintainers to weigh in.

Still opposed to this along with #3096951: d7_node migration should have dependency on d7_node_title_label migration It is confusing dependencies, which is for when migrations MUST run in a certain order or they will fail, with a series of migrations needed to accomplish some higher-level migration task, all of which must run for the task to be complete, but order does not affect the final state.

Whether you run d7_custom_block followed by block_content_entity_display or block_content_entity_display followed by d7_custom_block, your system is in the same state. The fact that you need both to do the comparison you want to doesn't make one a dependency of the other.

I'll leave this and the other open for other comments for now, but will likely close both as 'works as designed' in another week or so.

Not sure adding optional dependencies is a good idea or not

I thought for a while that adding these as optional would be the right way to go. But after more thought and looking at some of the 25 migrations in core that use optional dependencies I think not. The optionals in d7_node do make a difference to the final state if they are run after d7_node whereas the optionals in this patch don't.

So, in this case I think the optionals are not needed.