UNPKG

7.61 kBMarkdownView Raw
1# Guidelines for contributing to trace-nodejs
2
3RisingStack welcomes contributions for the trace-nodejs package by anyone who'd
4like to make it better!
5
6To make everyone's life easier, please follow this guidelines on how you can do
7that.
8
9## Legal
10
11By sending pull request you agree to grant RisingStack, Inc. perpetual,
12irrevocable, non-exclusive, worldwide, no-charge, royalty-free, unrestricted
13license in the code, algorithms, idea, object code, patch, tool, sample,
14graphic, specification, manual or documentation (“Your Contribution”) to
15reproduce, publicly display, publicly perform, sublicense, and distribute Your
16Contribution and such derivative works. You provide your contributions on an "as
17is" basis, without warranties of any kind, including warranties of correctness
18or fitness for a particular purpose.
19
20## Code compatibility
21
22The library code is required to run on target platforms Node.js >=v0.10 (Node >=
23v0.12 for 3.x.x) without transpilation. This means the code should fully adhere to the
24ES5 standard: **Use only ES5 compatible language features**.
25
26Most notable gotchas without completeness:
27
28 - No classes with `class`
29 - No arrow functions
30 - No object method shorthand
31 - No property assignment shorthand
32 - No rest parameters, no destructuring
33 - A lot of `Array` and `Object` methods you have in ES6 are missing from earlier versions.
34
35> When you are in doubt about the compatibility of a language specific feature check on [Mozilla Developer Network](mdn)!
36
37Moreover, the code must be compatible with target Node.js
38APIs. As these remain mostly backward compatible, (functionality is added,
39seldom removed) this boils down to finding the common subset of features and in
40turn looking at the earliest supported version's feature set.
41
42A few points to mention:
43 - Sync APIs were introduced in v0.12, v0.10 misses these.
44 - Streams API were largely extended in later versions.
45
46## Source code style
47
48We use [ESLint](eslint) for code linting.
49
50### ESLint config
51
52We use an ESLint configuration called [Standard Style](stdjs).
53
54An incomplete list of rules:
55 - 2 spaces – for indentation
56 - Single quotes for strings – except to avoid escaping
57 - No unused variables – this one catches tons of bugs!
58 - No semicolons
59 - Never start a line with (, [, or \` This is the only gotcha with omitting
60semicolons – automatically checked for you!
61 - Space after keywords
62 - Space after function name
63 - Always use `===` instead of `==` – but `obj == null` is allowed to check `null || undefined`.
64 - Always handle the node.js `err` function parameter
65
66### Functional vs imperative
67
68Code clarity is an important to us. Immutable data provides easier effect tracking
69therefore lesser cognitive overhead when understanding code of others. The
70fundamental program composition model of functional programming is based on
71function composition rather than state. However, keep in mind that Javascript is
72not a functional language. Using idiomatic language constructs even in presence
73of side effects enhances collaboration, and helps writing performant code. This
74requires you to keep a balance of imperative and functional code. The following
75rule of thumbs will help you:
76
77 - encapsulate and segregate state. Don't keep
78stateful objects all over the place, rather in reservoirs. Use private
79properties (starting with an \_underscore) to encapsulate state that should not
80be accessed from outside.
81
82 - use resourceful state in a controlled manner, for
83example provide state management functions for native timers.
84
85 - Keep in mind that function calls in instrumentation wrapper code will be
86 present in the stack trace if an error happens in the target library. So
87 - keep function call chains as shallow as possible.
88
89### Error tolerant instrumentation code
90
91It is very important that the
92instrumentation hooks are tolerant to misuse. We should expect errors to be
93present in user code interfacing with the instrumented libraries. In this case
94it should be left to the instrumented library to handle the error, so that our
95intermediate layer remains as transparent as possible.
96
97## Code organization
98```
99.
100├── CHANGELOG.md
101├── CONTRIBUTING.md
102├── LICENSE
103├── README.md
104├── circle.yml
105├── coverage
106├── example There is an example app here.
107├── lib Source code goes here, along with unit tests.
108├── package.json
109├── scripts Build, test, deployment scripts.
110└── test All tests except unit tests.
111```
112
113Keep the unit tests next to their corresponding source files. This makes it
114easier to navigate between them.
115
116## Guidelines about package dependencies
117
118Keep in mind at all times that the code will run on clients' machines. Every
119dependency consumes some disk space, adds memory overhead (when loaded), network
120overhead on install. Native modules make build times longer with a
121non-negligible amount (if they are allowed to be built at all). Additionally
122every new dependency or update adds the possibility to introduce a bug, a
123regression or a security hole. To minimize exposure to such adverse effects,
124following rules must be adhered to.
125
126### Evaluation
127
128Before adding a dependency first ask yourself:
129
130#### How does this package help?
131
132Summarize why this particular package helps solving your problem.
133
134#### Is it really needed, can I achieve my goal without it? If I can what are the
135trade-offs (very complex, missing knowledge in domain)?
136
137Some packages do not simplify your problem too much. Those that add syntactic
138sugar for control structures, validation shortcuts, or those that try to solve a
139very complex general problem that you are facing may not be the best candidate
140for inclusion. However if it solves a specific hard mathematical or domain
141specific problem, eg. cryptographic or compression algorithms, protocol
142implementations, use it. Another particular kind is polyfills and lodash-like
143helpers, these are generally acceptable, just use them if you really need to.
144
145#### Can I find a more mature one? Better maintained, used by more projects, etc.
146
147This should go without further explanation.
148
149#### Is there a package covering my needs with smaller footprint? Less transitive dependencies, optional native deps, etc.
150
151Prefer modular packages, avoid kitchen sinks. For example, use lodash.find and
152lodash.defaults instead of depending on the whole lodash module, if you need
153only those two methods.
154
155Prefer flat packages, that is, avoid ones with big dependency subtrees. Remember the leftpad scandal.
156
157#### Is it a native module or does it include a native dependency? Is it optional?
158
159To build native dependencies the native compiler build chain is required as well
160as some additional dependencies. Often on production systems these are not
161present because of security reasons. Prebuilding can be done but complicates
162distribution, therefore currently we do not do it. This means currently every
163native dependency must be optional, so the software needs to function (possibly
164with limited feature set and/or performance) without them.
165
166### Adding the dependency
167
168**Lock the dependency!** Production dependencies must be locked in package.json. This means that semver
169ranges are not allowed. The reason behind this is our experience with
170change management in npm packages. There is a possibility that they introduce
171breaking changes even on patch level, and it is always harder to provide support
172for a range of possible versions.
173
174[mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript
175[eslint]: http://eslint.org/
176[stdjs]: http://standardjs.com/