-
Notifications
You must be signed in to change notification settings - Fork 10
Feature/services user realms #100
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
Conversation
@@ -42,37 +48,98 @@ describe('user', function () { | |||
}); | |||
} | |||
|
|||
beforeEach(function (done) { | |||
dao.deleteAllUsers(function (err) { | |||
function configOptions(port, path, method, body, auth, version) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Node (or JavaScript in general), if you have a function with lots of input parameters it may be considered a code-smell. Its really hard to read function calls like configOptions('/test', 'GET', true, null, false, true)
. I'd rather have calls like this:
configOptions({
url: '/test',
method: 'GET',
body: {/* some object */},
auth: false,
version: true
});
Of course, this became obvious only after you made this refactor. And we have worst duplication problems. So, 👍 to you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right thx for the catch up. I'll change its signature as soon as i have 5 free mins 👍
Merged into #103 |
Add services (both public and internal) to add or remove user realms.