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

Tight coupling between embedded scripts and default versioning #43

Open
mattiasnordqvist opened this issue Sep 15, 2019 · 2 comments
Open

Comments

@mattiasnordqvist
Copy link
Owner

Found this code in DefaultMigrationVersioning:
private Regex embeddedScriptNameVersionRegexp = new Regex(@"\._(?<maj>\d{1,})\._(?<min>\d{1,})\.");

Seems to indicate some coupling between versioning and embedded scripts. This needs to be fixed to allow easier adding of new script providers.

@mattiasnordqvist
Copy link
Owner Author

mattiasnordqvist commented Sep 15, 2019

How can we fix this? We want to be able to decide a versioningscheme for all our scripts, be it embedded resources or C# classes. The current implementation of DefaultMigrationVersioning however, expects the Script name to bear resemblence with the name of an embedded resource (with underscores and dots). If I implement some IScript in code, I don't want the Name-property to have to conform to this strange naming convention.

One solution:
When it comes to IScript, I think we simply need to extract the Version part from the Name property, and put it in a seperate property. (Maybe Name can stay as it is) In the case of EmbeddedScript, this means we need to extract the version from the resource-key, without really knowing exactly what the version might look like (but we know it should be the name of its parent folder).

Another solution:
The migrationversioning need to differentiate between different type of scripts based on their script class maybe, or some new "Source"-property or something. We can achieve this by letting the migrationversioning have a dictionary keyed on ScriptClass/Source, with values being function to extract the version from the name. Every time we add a new class of scripts, this function needs to be registered with the migrationversioning class.

@mattiasnordqvist
Copy link
Owner Author

Will need your input here @carl-berg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant