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

Add option to modify embedded script names via ScriptNameProvider #737

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

osexpert
Copy link

@osexpert osexpert commented Sep 30, 2023

Solve #166 and #734

Can use like:

builder.WithScriptsEmbeddedInAssemblies(Assembly.GetExecutingAssembly(), options => 
{
 options.ScriptNameFromResourceName = EmbeddedScriptsOptions.FileNameFromResourceName;
});

This extension method follow the same configure logic seen in AspNet, and with this, only one WithScriptsEmbeddedInAssemblies method is needed and is easily extendable with extending EmbeddedScriptsOptions instead of adding new extension methods.

@osexpert
Copy link
Author

So if this PR get approved, I will suggest to remove all\most other WithScriptsEmbeddedInAssemblies extension methods in the next breaking version.

@osexpert
Copy link
Author

My initial idea was to make configure optional (configure = null) but then the extension methods may collide with these two:

public static UpgradeEngineBuilder WithScriptsEmbeddedInAssemblies(this UpgradeEngineBuilder builder, Assembly[] assemblies)
public static UpgradeEngineBuilder WithScriptsEmbeddedInAssembly(this UpgradeEngineBuilder builder, Assembly assembly)

But in the next breaking version, configure can be made optional (configure = null) and then remove the other two.

Add a system delivered ScriptNameFromResourceName function: EmbeddedScriptsOptions.FileNameFromResourceName

remove excessive ws
do not throw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Wishlist - Naming of Scripts
Development

Successfully merging this pull request may close these issues.

1 participant