-
Notifications
You must be signed in to change notification settings - Fork 18
Conversation
Initial commit
Fix packaging error
Fix handling of empty body with description
Update fury to v1.0.3
Update to Fury 2.3
chore: Upgrade to peasant 1 and release 0.3.0
chore: Upgrade peasant to 1.1.0
Update to fury@3
Support fury 3.0.0-beta.3
Add support for some of the data structures produced from the Swagger adapter
Support Fury 3.0.0 Beta 4
Release 0.6.0
Test on Node 8 and 10
Update to fury 3.0.0-beta.7
Update .gitignore
chore: Update dev dependencies
Fix CHANGELOG format consistency
a2a0d7c
to
b0022be
Compare
* Indent a piece of multiline text by a number of spaces. | ||
* Setting `first` to `true` will also indent the first line. | ||
*/ | ||
const indent = (input, spaces, options = { first: false }) => { |
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.
@pksunkara why did you make these closures instead of functions?
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.
Because we are exporting these in this module. I generally use the rule that if I am exporting functions in ES6, I make them closures
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.
This brings inconsistency, against both other packages we maintain and even other files in the same package (function getTypeAttributes(element, attributes, parent) {
a few lines down). That said, I believe it should be a named function for numerous reasons, due to scoping, hoisting, and transferring of this
. Our style guide is also clear on this, it should be a named function instead of a function declaration:
7.1:
7.1 Use named function expressions instead of function declarations. eslint:
func-style
Why? Function declarations are hoisted, which means that it’s easy - too easy - to reference the function before it is defined in the file. This harms readability and maintainability. If you find that a function’s definition is large or complex enough that it is interfering with understanding the rest of the file, then perhaps it’s time to extract it to its own module! Don’t forget to explicitly name the expression, regardless of whether or not the name is inferred from the containing variable (which is often the case in modern browsers or when using compilers such as Babel). This eliminates any assumptions made about the Error’s call stack. (Discussion)
// bad function foo() { // ... } // bad const foo = function () { // ... }; // good // lexical name distinguished from the variable-referenced invocation(s) const short = function longUniqueMoreDescriptiveLexicalFoo() { // ... };
Arrow functions are function expressions instead of named functions. You cannot declare a named arrow function.
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.
I have been doing this for other packages in the monorepo. I will make a separate PR standardizing this once this is merged.
Fixes #33