Skip to content

Conversation

@fanatid
Copy link
Member

@fanatid fanatid commented Apr 25, 2016

No description provided.

@fanatid
Copy link
Member Author

fanatid commented Apr 25, 2016

@dcousens I change conditions in encode, need patch (minor?) release.

check: check,
decode: decode,
encode: encode
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I prefer the module.exports at the end approach... easier to visualize.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dcousens then, let's back to this code. What do you think about export right after require?

Copy link
Contributor

@dcousens dcousens Apr 26, 2016

Choose a reason for hiding this comment

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

@fanatid can't stand it.
I follow these three rules which lead to the result I typically have:

  • No use before declaration
  • No repeated declarations
  • Prefer named functions
  • Group hackish/lame/ugly code for easy removal one day

Moving the exports to the top breaks my first rule.
The current implementation breaks my second rule, and removing the function name makes it an anonymous function (ew).
The previous implementation met all these rules.

Ideally, ES Next's export style syntax would be available... until then, I use the module.exports = {} at EOF.

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

@dcousens thanks for published your rules, it's really helpful.
I'm ok with them, do you need PR for module.export or you do this yourself?
Can you publish new version after this?

@dcousens dcousens merged commit a03399d into master Apr 26, 2016
@dcousens dcousens deleted the feature/revert branch April 26, 2016 01:18
@dcousens
Copy link
Contributor

Thanks @fanatid

dcousens added a commit that referenced this pull request Apr 27, 2016
fanatid added a commit that referenced this pull request Apr 27, 2016
index: revert #5 encode changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants