Skip to content
This repository was archived by the owner on Jun 25, 2020. It is now read-only.

Conversation

@laedit
Copy link
Contributor

@laedit laedit commented Dec 13, 2015

Fix #285
Configuration is now accessible through MEF as IConfiguration.

Jérémie Bertrand added 2 commits December 13, 2015 18:44
Configuration is now accessible through MEF as IConfiguration.
@laedit
Copy link
Contributor Author

laedit commented Dec 13, 2015

@thoemmi I hadn't the time to test it in a real plugin but this should work.
Could you tell me if it's ok for you?

@thoemmi
Copy link
Contributor

thoemmi commented Dec 13, 2015

@laedit I tried it in my plugin, it works great! 👍 :shipit:

@thoemmi
Copy link
Contributor

thoemmi commented Dec 13, 2015

Well, my other plugin crashes when I don't re-compile it. It uses this code

if (!context.Config.TryGetValue("redirect_generate_aspx", out generateAspx)) {
    return false;
}

which will throw System.MissingMethodException:

Additional information: Method not found: 'System.Collections.Generic.IDictionary`2<System.String,System.Object> Pretzel.Logic.Templating.Context.SiteContext.get_Config()'.

SiteConfig.Config returns now an instance of IConfiguration instead of IDictionary<string,object>. I don't know if this is a showstopper, because it may break existing plugins.

@laedit
Copy link
Contributor Author

laedit commented Dec 13, 2015

I know that this is a breaking change, but I prefer to keep the config read-only instead of letting it as a dictionary.
If you are ok with that I will merge the branch this week.

@thoemmi
Copy link
Contributor

thoemmi commented Dec 13, 2015

I'm fine with that, I just wanted to make you aware of the issue. I'm looking forward to the merge.

Are you going to publish a new release and NuGet package once you've merged the changes?

@laedit
Copy link
Contributor Author

laedit commented Dec 14, 2015

I plan to make a new release in january, I won't have the time to do it until then and I'm waiting that the chocolatey packages are validated.

laedit pushed a commit that referenced this pull request Dec 14, 2015
@laedit laedit merged commit de2a8b5 into master Dec 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants