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

module cleanup and ngdoc #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mangei
Copy link

@mangei mangei commented Sep 23, 2015

module definitions are always on top and also all dependencies ("imports")

@tsalzinger
Copy link
Contributor

why?

there was an intention about having the module configuration at the bottom:

  • don't reference methods before they are defined
  • have the important stuff at the top (the actual implementation, instead of the angular metadata)

both reasons are open for discussion but i don't see why we would need to change this

@mangei
Copy link
Author

mangei commented Sep 24, 2015

I see your points

don't reference methods before they are defined

In Javascript is does not matter if a function is defined before or after it is used if it is within the same scope. I prefer it to have it on top, to easily see the module definition and its dependencies (like in other programming languages, where the imports/dependencies are always on top)

have the important stuff at the top (the actual implementation, instead of the angular metadata)

this meta information is not very big (usually just some lines), but important. having to scroll to the bottom of the file is to "find" the declaration is in my eyes not very convenient.

This is why I think it is better to have them on top instead at the bottom.

@tsalzinger
Copy link
Contributor

i know that js dosn't care, but i do

it is even checked via jshint

please ensure that you have jshint enabled

@tsalzinger
Copy link
Contributor

to ensure all changes adhere to the jshint config i just included it into the build pipeline

@tsalzinger
Copy link
Contributor

ping

@szabyg
Copy link

szabyg commented Feb 29, 2016

This is why I think it is better to have them on top instead at the bottom.

I agree, but for sure this is a matter of what you're used to see and a matter of personal subjective preference. In this sense, for me: The "metadata" is the first thing I'd want to see in an angular file: What is this file doing in my code? Is this a service/factory/controller/directive? Under what name can I use it? How it is implemented is something I'd might want to know afterwards, in the knowledge of what it is.

Ad JSHint: True but it's an optional check. You can disable it with "latedef": false in .jshintrc.

@tsalzinger
Copy link
Contributor

Ad JSHint: True but it's an optional check. You can disable it with "latedef": false in .jshintrc.

of course it is, JSHint is optional in general I could just remove it completely, but it is here for a reason and I won't change the configuration

you can configure it however you want in your codebase, but if you contribute to a open source project you have to follow their rules, not your own

therefor this point is not open for discussion

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.

3 participants