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

Allow ordering of WebActivator calls across multiple assemblies #13

Closed
wants to merge 1 commit into from

Conversation

neilduncan
Copy link
Contributor

Hi David,

I have some web activator calls which live in a number of libraries. At the moment, Web Activator ordering only orders the calls inside a single assembly, not across multiple.

I think what I've done here should solve this. Do you want to take a look?

Thanks,
Neil

@davidebbo
Copy link
Owner

I'm out and won't be able to look at this in depth till August.

Though I do find it a little odd to order across assemblies in the sense that there is no shared understanding of the Ordering even means when assemblies are authored by different people.

@neilduncan
Copy link
Contributor Author

Yeah, I can understand that point of view. It's not something that I'd
expect many people to run up against. I've certainly been using
WebActivator for a while without needing it!

Perhaps some more about the use case might be helpful?

We've got an internal product that we maintain, and which gets dropped into
various websites as a Nuget package. The product initialises itself using
WebActivator. We now need the ability to customise that initialisation per
website, which is why I'm concerned about the ordering across assemblies.

What about looking at it the other way? Could you see a downside to
ordering across assemblies?

On Friday, July 12, 2013, David Ebbo wrote:

I'm out and won't be able to look at this in depth till August.

Though I do find it a little odd to order across assemblies in the sense
that there is no shared understanding of the Ordering even means when
assemblies are authored by different people.


Reply to this email directly or view it on GitHubhttps://github.com//pull/13#issuecomment-20905679
.

@davidebbo
Copy link
Owner

See related discussion in #10. We should try to get your change in, though I probably won't be able yo get to it for another week. To answer your question, I don't really see a downside, other than potentially breaking scenarios that were making questionable assumption about the order they currently get.

Hopefully, in scenarios where people don't use Order attributes, your change will have no effect on the effective ordering.

@neilduncan
Copy link
Contributor Author

Great. The commit looks a bit big, but most of the changes are just adding
a new assembly for testing.
The actual change to the code is tiny.

On Wed, Jul 31, 2013 at 9:17 AM, David Ebbo notifications@github.comwrote:

See related discussion in #10#10.
We should try to get your change in, though I probably won't be able yo get
to it for another week. To answer your question, I don't really see a
downside, other than potentially breaking scenarios that were making
questionable assumption about the order they currently get.

Hopefully, in scenarios where people don't use Order attributes, your
change will have no effect on the effective ordering.


Reply to this email directly or view it on GitHubhttps://github.com//pull/13#issuecomment-21846879
.

@davidebbo
Copy link
Owner

Yes, the change looks safe. Would you mind checking that the sorting algorithm is stable? That would guarantee that the ordering will not be affected in the (common) case where no Order is specified.

@davidebbo
Copy link
Owner

Ok, I see @jskeet says here that it's stable, so that's good enough for me!

@davidebbo
Copy link
Owner

Could you yank the ncrunch file and resubmit (maybe add to gitignore)? Please reset so the file is not in the history line. Thanks!

@neilduncan
Copy link
Contributor Author

Argh. Yes, will do.

On Tuesday, 6 August 2013, David Ebbo wrote:

Could you yank the ncrunch file and resubmit (maybe add to gitignore)?
Please reset so the file is not in the history line. Thanks!


Reply to this email directly or view it on GitHubhttps://github.com//pull/13#issuecomment-22203537
.

@davidebbo
Copy link
Owner

Actually, I just did it myself. The change is in! Please try WebActivatorEx 2.0.3 to make sure everything looks good. Thanks for your contribution!

@davidebbo davidebbo closed this Aug 6, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants