Update for Contributors library guidelines#195
Conversation
maxdeviant
left a comment
There was a problem hiding this comment.
LGTM 👍
Should we open an issue for cleaning up the eslint errors?
|
I'd like to briefly check in with @natefaubion and hear how he'd like to handle it. I think ideally we would merge this pull request, then open a PR which addresses the eslint warnings and errors -- perhaps modifying the eslint configuration if needed. @natefaubion knows much, much more about the Aff internals than I do so this configuration should really come down to his decision. |
|
As far as eslint goes, I would prefer to keep the source as is and not make unnecessary stylistic changes just for the sake of it. If there ever comes a time to do significant refactoring we can take that as an opportunity to be more consistent. |
|
The eslint rules which would otherwise cause a failed build have been disabled in the source (but the configuration has been kept very close to the standard one). We can update the source itself when there is reason to. |
This pull request is part of an effort to update and standardize the Contributors libraries according to the Library Guidelines. Specifically, it:
This is a first step towards ensuring Contributors libraries have adequate module documentation, READMEs, a docs directory, and tests (even if just usage examples) in a
testdirectory.In this library's case, there's one more step to take. I applied the standard eslint configuration, which was initially developed using the rules from this repository and covers the same rules from the jshint configuration currently here. This configuration is a little stricter and has caught several (potential) issues in the code:
FIBER,THUNK, some local variables (ie. "i")These merit an independent discussion and review aside from this PR, which doesn't touch the code other than to disable eslint so we can have that separate discussion.