keon94 commented on PR #5606: URL: https://github.com/apache/incubator-devlake/pull/5606#issuecomment-1614622350
> This approach would work temporarily, but it is not forward-compatible. > > Migration script should be self-contained and consistent, meaning it should not be using any models outside the module of itself because they could be changed over time, which changes the behavior of the script. That is why we copying the original `models` as `migrationscripts/archived`. > > Another problem is that we should not do it in the Framework level: what if we compile only jira and boot up the system, now the migration script gets applied, and it won't get executed again if I do it for github next time. > > I would suggest that we do it in one plugin one script manner without using the interface(it might change in the future as well). copy the struct if it is needed by the script. Actually, as far as compilations, the Name() of the script contains the plugin name, meaning if we boot up the system with a new plugin it should detect the missing migration for that plugin based on the name. If you look at the migration history table, it is adding one record per plugin to it. Perhaps this logic can be moved to a different package and not live in core (e.g. in helper/), but I highly prefer having something like this to reduce boilerplate and redundancies in our codebase. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
