Skip to content

Conversation

@remcohaszing
Copy link
Collaborator

I think these patches uncover some issues with the utils. Also the utils are untested.

I would like to make tests for them and fix those issues if I'm sure I've got the documentation right.

Previously all was directly assigned to the module.exports object.

Now it is somewhat more structured.

First properties are listed.
Next modules.exports is defined, giving an overview of the module.
Last named functions are declared. This provides better stacktraces.
Copy link
Owner

Choose a reason for hiding this comment

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

If these are not considered as service, I think this is a mistake. We should update this isAngularServiceDeclaration method in order to support these exta syntaxes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's why I wanted someone else to take a look at the docs I added and see if they are correct. If these docs are correct I would like to add tests and fix the issues I marked with @todo FIXME.

@EmmanuelDemey
Copy link
Owner

for me, the doc looks good.

These tests uncover some issues that need to be looked over.
@tilmanschweitzer
Copy link
Collaborator

looks good for me too. I'll merge it.

tilmanschweitzer added a commit that referenced this pull request Oct 13, 2015
@tilmanschweitzer tilmanschweitzer merged commit c2e1435 into EmmanuelDemey:development Oct 13, 2015
@remcohaszing remcohaszing deleted the utils-restructure branch December 9, 2015 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants