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

configurable user collection #27

Merged
merged 4 commits into from
Dec 21, 2016
Merged

Conversation

haamond
Copy link
Contributor

@haamond haamond commented Dec 20, 2016

allow users to pass an optional user's collection name to MongoUserStore constructor. #26

@@ -23,6 +23,8 @@ bld/

# Visual Studio 2015 cache/options directory
.vs/
# Visual Studio Code / options directory
.vscode
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be ignored. This folder is from VS Code to hold workspace specific settings.

@@ -35,7 +35,7 @@ static MongoUserStore()
MongoConfig.EnsureConfigured();
}

public MongoUserStore(IMongoDatabase database, ILoggerFactory loggerFactory)
public MongoUserStore(IMongoDatabase database, ILoggerFactory loggerFactory, string userCollectionName = "users")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you go with constructor overload instead of optional parameters?

see http://haacked.com/archive/2010/08/10/versioning-issues-with-optional-arguments.aspx/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of course, nice article :)

Copy link
Owner

@tugberkugurlu tugberkugurlu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SorenZ Thanks for the PR 😄 Seems generally good, there are couple minor things to change. If you can change them, I will get this in 😄

@haamond
Copy link
Contributor Author

haamond commented Dec 21, 2016

@tugberkugurlu I hope the changes accepted.
I learned nice things from your suggestions.
look forward to hear from you.

@tugberkugurlu
Copy link
Owner

@SorenZ great, thanks a lot! :shipit:

@tugberkugurlu tugberkugurlu merged commit 0657852 into tugberkugurlu:master Dec 21, 2016
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