WSOD when installing on D9

tra.duong → made their first commit to this issue’s fork.

I have check the commit @richgerdes's commit and it removed the BSOD.

However, depends on https://www.drupal.org/docs/drupal-apis/entity-api/converting-a-content-... →
I also add a hook_update_N to migrate old data just in case.

I put an MR.

@tra.duong, i looked at your MR, but its hard to tell what was actually changed because you reset formatting on the entire file. Would it be possible for you to split the MR up into two commits, one to include the general formatting updates and one for the update hook you added so it can be easily reviewed?

@richgerdes
I have check the commit again, it makes me confuse somehow.
https://git.drupalcode.org/project/event/-/merge_requests/2/diffs
Please uncheck "Show whitespace changes" in "Preferences", the whitespaces may auto-corrected by my IDE.

Just a minor comment on this change.

The function `event_update_dependencies` has been added between `event_update_8102` and the new `event_update_8103`.

Just from a viewpoint of making changes easy to follow, I think the `event_update_` functions should be grouped together, and the `event_update_dependencies` function should be placed either at the top or the bottom of the file.

I'm haven't been involved in this module, but I'd be concerned about an update requiring "Show whitespace changes" to be turned on. I can't see an obvious reason for the whitespace to have changed.

Typically a clean commit should be as simple as possible, only changing the lines required to fix the bug. In this case the MR changes the line endings of the install file form the unix format (only \n) to the windows format (\r\n). This is an easy enough to ignore in the editor but it does clutter the git log and make it harder to trace the cause of a bug in the blame history. Best practice is typically to make a change like that as its own commit.

In this case, its not necessary and would deviate from the established standard of the repository. I've reverted the line endings changes in a new commit on the branch.

Otherwise, I think that the two hooks being added are good, and will benefit users of the module. As @lostcarpark, event_update_dependencies is more of an informational hook, instead of a update process, so its ideal that they are kept together. I've also moved the change to the top of the file.

Otherwise everything looks good to me. @tra.duong, I really appreciate the through and thorough work here to get the hook added.

Automatically closed - issue fixed for 2 weeks with no activity.