on c94f540 help sign in
streamly/docs/Developer/Contributing.md
272 lines 11 kB history raw
1# Contributors' Guide
3## Bug Reports
5Please feel free to [open an
6issue](https://github.com/composewell/streamly/issues) for any questions,
7suggestions, issues or bugs in the library. In case you are reporting a bug
8we will appreciate if you provide as much detail as possible to reproduce or
9understand the problem, but nevertheless you are encouraged to open an issue
10for any problem that you may encounter.
12## Picking Issues to Work on
14Beginners are encouraged to pick up issues that are marked `help wanted`. It is
15a good idea to update the issue expressing your intent so that others do not
16duplicate the effort and people with a background on the issue can help.
18## Build and test
20Some common commands:
22```
23$ cabal repl
24$ cabal build
25$ cabal build --disable-optimization --flags -opt
26$ bin/test.sh --quick
27$ bin/bench.sh --quick # For nix use "--use-nix"
28$ bin/run-ci --targets help # Only supported with nix
29$ cabal haddock
30$ cabal-docspec --timeout 60 --check-properties --property-variables xs
31```
33The `bin/test.sh` and `bin/bench.sh` are custom scripts to perform test
34and benchmarking tasks conveniently. They support running a group of
35tests or benchmarks, coverage, using correct optimization flags for
36benchmarking, benchmark comparison reports etc. Use the `--help` option
37to see how to use these. See [benchmark/README.md](/docs/Developer/Benchmarks.md)
38and [test/README.md](/docs/Developer/Tests.md) for more details.
40You can also run tests or benchmarks directly for example:
42```
43$ cabal test --disable-optimization --flags -opt --test-show-details=streaming --test-option=--color streamly-tests
44```
46To view haddock docs open the link printed at the end of the haddock
47command, in your browser.
49Nix users: There is a `default.nix` available which can be used to start
50a nix-shell and then use the above commands as usual. nix-shell started
51by `default.nix` creates a `.cabal.nix` for cabal configuration and sets
52`CABAL_DIR` environment variable to point to that. Therefore, `~/.cabal`
53would not be used.
55## Contributing A Change
57If the feature makes significant changes to design, we encourage you to open an
58issue as early as possible so that you do not have to redo much work because of
59changes in design decisions. However, if you are confident, you can still go
60ahead and take that risk as the maintainers are supposed to be reasonable
61people.
63### Pull Requests (PR)
65Please feel free to [send a pull
66request (PR)](https://github.com/composewell/streamly/pulls) whether it is a
67single letter typo fix or a complex change. We will accept any PR that makes a
68net positive change to the package. We encourage you to provide a complete,
69consistent change with test, documentation and benchmarks. You can contact the
70[maintainers](https://gitter.im/composewell/streamly) for any help or
71collaboration needed in that regard. However, if due to lack of time you are
72not able to complete the PR, you are still welcome to submit it, maintainers
73will try their best to actively contribute and pick up your change as long as
74the change is approved.
76### Pull Request (PR) Checklist
78Here is a quick checklist for a PR, for details please see the next section:
80* PR contains one logical changeset
81* Each commit in the PR consists of a logical change
82* Commits are rebased/squashed/fixup/reordered as needed
83* Stylistic changes to irrelevant parts of the code are separated in
84 independent commits.
85* Code is formatted as per the style of the file or that of other files
86* Compiler warnings are fixed
87* Reasonable hlint suggestions are accepted
88* Tests are added to cover the changed parts
89* All [tests](/test) pass
90* [Performance benchmarks](/benchmark) are added, where applicable
91* No significant regressions are reported by [performance benchmarks](/docs/Developer/Benchmarks.md)
92* Haddock documentation is added to user visible APIs and data types
93* Tutorial module, [Introduction](/docs/User/Tutorials/Introduction.md), and [guides](/docs) are
94 updated if necessary.
95* [Changelog](/docs/User/ProjectRelated/Changelog.md) is updated if needed
96* The code conforms to the license, it is not stolen, credit is given,
97 copyright notices are retained, original license is included if needed.
99### Structuring Pull Requests
101__Lean PR:__ Please keep the reviewer in mind when sending pull
102requests. Use one PR for each logical changeset. A lean and thin
103PR with one independent logical change is more likely to be reviewed and
104merged quickly than a big monolithic PR with several unrelated changes.
106__PR Dependencies:__ If your change depends on an earlier PR you can
107create a branch from the old PR's branch, and raise a new PR targeting
108to merge into the old PR branch. Do not push the commits to the same PR
109just because the commit depends on that PR.
111__Commits:__ You are encouraged to group a logically related set of
112changes into a single commit. When the overall changeset is largish you
113can divide it into multiple smaller commits, with each commit having
114a logically grouped changeset and the title summarizing the logical
115grouping. Always keep in mind a logical division helps reviewers
116understand and review your code quickly, easier history tracking and
117when required clean reversal changes.
119__Functional Changes:__ Keep the reviewer in mind when making
120changes. Please resist the temptation to make style related changes to
121surrounding code unless you are changing that code anyway . Whenever
122possible, try to separate unrelated refactoring changes which do not
123affect functionality in separate commits so that it is easier for the
124reviewer to verify functional changes.
126__Style Changes:__ Make sure that your IDE/Editor is not automatically
127making sweeping style changes to all the files you are editing. That
128makes separating the signal from the noise very difficult and makes
129everything harder. If you would like to make style related changes
130then please send a separate PR with just those changes and no other
131functionality changes.
133__Rebasing:__ If your commits reflect how you fixed intermediate
134problems during testing or made random changes at different times you
135may have to squash your changes (`git rebase -i`) into a single commit
136or logically related set of commits.
138### Resolving Conflicts
140If during the course of development or before you send the PR you find that
141your changes are conflicting with the master branch then use `git rebase
142master` to rebase your changes on top of master. DO NOT MERGE MASTER INTO YOUR
143BRANCH.
145### Testing
147It is a good idea to include tests for the changes where applicable. See
148the existing tests in the [test](/test) directory. Also see the
149[test/README.md](/docs/Developer/Tests.md) for more details on how to run the tests.
151### Documentation
153For user visible APIs, it is a good idea to provide haddock documentation that
154is easily understood by the end programmer and does not sound highfalutin,
155and preferably with examples. If your change affects the tutorial or needs to
156be mentioned in the tutorial then please update the tutorial. Check if the
157additional [guides](/docs) are affected or need to updated.
159### Performance Benchmarks
161It is a good idea to run performance benchmarks to see if your change affects
162any of the existing performance benchmarks. If you introduced something new
163then you may want to add benchmarks to check if it performs as well as expected
164by the programmers to deem it usable.
166See the [README](/docs/Developer/Benchmarks.md) file in the `benchmark` directory for more
167details on how to run the benchmarks.
169The first level of check is the regression in "allocations", this is
170the most stable measure of regression. "bytesCopied" is another stable
171measure, it gives an idea of the amount of long lived data being copied
172across generations of GC.
174The next measure to look at is "cpuTime" this may have some variance
175from run to run because of factors like cpu frequency scaling, load on
176the system or the number of context switches etc. However, the variance
177would usually be within 5-10%, anything more than that is likely to be a
178red flag.
180Quite often an increase in "cpuTime" would correspond to an increase
181in "allocations". Increase in "allocations" indicates an inefficiency
182in computing due to too much GC activity e.g. due to lack of
183fusion. However, it is also possible for the cpu time to go up without
184the allocations going up, this indicates an inefficiency in processing
185in general or more CPU being used for the same task. It could be because
186of inefficient code generation, branch mis-predictions or lack of cache
187locality etc.
189For concurrent benchmarks we can compare "cpuTime" and "time" to check
190the degree of concurrency and total efficiency.
192### Changelog
194Any new changes that affect the user of the library in some way must be
195documented under `Unreleased` section at the top of the `Changelog`. The
196changes in the changelog must be organized in the following categories, in that
197order:
199* Breaking Changes
200* Enhancements
201* Bug Fixes
202* Deprecations
204If there are very few changes then you can just prefix a bullet with these
205annotations. If there are many changes make sections to group them. A section
206can be absent if there is nothing to add in it.
208If you make changes that are incompatible with the released versions
209of the library please indicate that in the `Changelog` as `Breaking Changes`
210and also write short notes regarding what the programmers need to do to adapt
211their existing code to the new change.
213### Licensing
215If you have copied code from elsewhere you need to conform to the
216licensing terms of the original code. Usually you would need to add a
217copyright notice in the source header, and include the license of the
218original code as per the terms of the license. If the code you are
219copying does not have an associated license please do not use it.
221## Developer documentation
223* Use https://streamly.composewell.com/ for searching in reference docs
224* Build haddock docs for latest reference docs
225* See [the dev directory](/docs/Developer) for design guidelines and dev documents.
227## Coding Guidelines
229### Coding Style
231Please see [the Haskell coding style guide](https://github.com/composewell/haskell-dev/blob/master/coding-style.rst).
233### Type variable ordering
235Ordering of variables in `forall` may be important in type applications. In
236streams, we usually declare them in this order `t m a`.
238Use the same order in constraints as well
240Example,
242```
243func :: forall m a. (MonadIO m, Storable a) => ...
244```
246### StreamD coding style
248Some conventions that we follow in the StreamD code are illustrated by the
249following example:
251```
252mapM f (Stream step1 state1) = Stream step state
253 where
255 step gst st = do
256 r <- step1 (adaptState gst) st
257 case r of
258 Yield x s -> f x >>= \a -> return $ Yield a s
259 Skip s -> return $ Skip s
260 Stop -> return Stop
261```
263* For the input streams use numbering for `step` and `state` e.g.
264 step1/state1. For the output stream use `step` and `state`.
265* For state argument of `step`, use `st`.
266* For result of executing a `step` use `r` or `res`
267* For the yielded element use `x`
268* For the yielded state use `s`
270In general, the rule is - the shorter the scope of a variable the shorter its
271name can be. For example, `s` has the shortest scope in the above code, `st`
272has a bigger scope and `state` has the biggest scope.