Skip to content

Commit e322f61

Browse files
committed
Address comments from code review.
1 parent bfc0ebb commit e322f61

File tree

1 file changed

+40
-36
lines changed

1 file changed

+40
-36
lines changed

CONTRIBUTING.md

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Contributing to `librustzcash` Crates
1+
# Contributing to Zallet
22

33
First off, thanks for taking the time to contribute! ❤️
44

@@ -140,7 +140,7 @@ Once it's filed:
140140
- If the team is able to reproduce the issue, it will be assigned an
141141
appropriate category and fixed according to the criticality of the issue. If
142142
you're able to contribute a proposed fix, this will likely speed up the
143-
process, although be aware that `librustzcash` is a complex project and fixes
143+
process, although be aware that Zallet is a complex project and fixes
144144
will be considered in the context of safety and potential for unintentional
145145
misuse of overall API; you should be prepared to alter your approach based on
146146
suggestions from the team and for your contributions to undergo multiple
@@ -197,18 +197,8 @@ Enhancement suggestions are tracked as [GitHub issues](/issues).
197197

198198
This repository is currently developed with an "unstable main" workflow. The
199199
current contents of the main branch is a preview of what the next full release
200-
of all crates may look like, but is not stable. For example, as-yet-unreleased
201-
`zcash_client_sqlite` migrations may be altered incompatibly at any time.
202-
203-
In the main branch, all crates have the version corresponding to their most
204-
recent stable release on https://crates.io; this enables the preview state to
205-
be tested ahead-of-time by downstream users via [patch.crates-io] directives.
206-
207-
Individual crates have their own tags, e.g. `zcash_primitives-0.19.0`. These
208-
tags point to the Git commit at which that crate version was published (which
209-
in general is not the merge commit for a release branch, but the actual commit
210-
that incremented the crate's version). Note however that other crates should
211-
not be considered stable at that revision.
200+
of all crates may look like, but is not stable. As-yet-unreleased code may be
201+
altered incompatibly at any time.
212202

213203
#### Merge Workflow
214204

@@ -259,9 +249,7 @@ conflicts.
259249
crates' `CHANGELOG.md` files to clearly document the change.
260250
- Updated or added members of the public API MUST include complete `rustdoc`
261251
documentation comments.
262-
- It is acceptable and desirable to open pull requests in "Draft" status. Only
263-
once the pull request has passed CI checks should it be transitioned to
264-
"Ready For Review".
252+
- Each commit should be formatted cleanly using `cargo fmt`.
265253
- There MUST NOT be "work in progress" commits as part of your history, with
266254
the following exceptions:
267255
- When making a change to a public API or a core semantic change, it is
@@ -277,8 +265,37 @@ conflicts.
277265
additions or other changes to the test framework may be required. Please
278266
consult with the maintainers if substantial changes of this sort are
279267
needed, or if you are having difficulties reproducing the bug in a test.
268+
- Each commit should pass `cargo +beta clippy --tests --all-features --all-targets`
269+
without warnings, excepting the first case described above for work-in-progress
270+
commits.
271+
272+
#### Pull Requests
280273

281-
#### Pull Request Review
274+
A pull request MUST reference one or more issues that it closes. Furthermore,
275+
DO NOT submit a pull request without a maintainer having acknowledged the
276+
validity of the issue(s) that the pull request purports to close.
277+
278+
It is acceptable and desirable to open pull requests in "Draft" status. Only
279+
once the pull request has passed CI checks should it be transitioned to "Ready
280+
For Review". Please @mention a maintainer if you need CI to be triggered in
281+
preparation for transitioning the PR to "Ready For Review"; CI does not run
282+
automatically for PRs from external contributors.
283+
284+
##### Commit Messages
285+
286+
- Commit messages should have a short (preferably less than ~120 characters) title.
287+
- The body of each commit message should include the motivation for the change,
288+
although for some simple cases (such as the application of suggested changes) this
289+
may be elided.
290+
- When a commit has multiple authors, please add `Co-Authored-By:` metadata to
291+
the commit message to include everyone who is responsible for the contents of
292+
the commit; this is important for determining who has the most complete
293+
understanding of the changes.
294+
- If any AI agent was used in writing the code being commited, you MUST
295+
maintain or add `Co-Authored-By:` metadata indicating the participation of
296+
the AI agent. Failure to do so is grounds for closing a pull request.
297+
298+
##### Pull Request Review
282299

283300
It is acceptable and desirable to use a rebase-based workflow within the
284301
context of a single pull request in order to produce a clean commit history.
@@ -345,19 +362,6 @@ missing cross-organization permissions); in this case (or if a user's PR has
345362
"allow maintainers to edit" disabled), we may close the PR and open a new PR
346363
containing the commits from the original.
347364

348-
#### Commit Messages
349-
350-
- Commit messages should have a short (preferably less than ~120 characters) title.
351-
- The body of each commit message should include the motivation for the change,
352-
although for some simple cases (such as the application of suggested changes) this
353-
may be elided.
354-
- When a commit has multiple authors, please add `Co-Authored-By:` metadata to
355-
the commit message to include everyone who is responsible for the contents of
356-
the commit; this is important for determining who has the most complete
357-
understanding of the changes. If any AI agent was used in writing the code
358-
being commited, you MUST maintain or add `Co-Authored-By` metadata indicating
359-
the use of the AI agent. Failure to do so is grounds for closing a PR.
360-
361365
### Coding Style
362366

363367
The Zallet authors hold our software to a high standard of quality. The list of
@@ -387,9 +391,8 @@ implications, including but not limited to the following:
387391
state space are used.
388392
- Use custom `enum`s with semantically relevant variants instead of boolean
389393
arguments and return values.
390-
- Prefer immutability; make data types immutable unless there is a strong
391-
reason to believe that values will need to be modified in-place for
392-
performance reasons.
394+
- Make data types immutable unless there is a strong reason to believe that
395+
values will need to be modified in-place for performance reasons.
393396
- Take care when introducing and/or using structured enum variants, because
394397
Rust does not provide adequate language features for making such values
395398
immutable or ensuring safe construction. Instead of creating structured or
@@ -405,8 +408,9 @@ This means:
405408
- Write referentially transparent functions. A referentially transparent
406409
function is one that, given a particular input, always returns the same
407410
output.
408-
- Avoid mutation whenever possible. If it's strictly necessary, use mutable
409-
variables only in the narrowest possible scope.
411+
- Avoid mutation. If it's necessary for performance, use mutable variables only
412+
in the narrowest possible scope (e.g. within the internal scope of a
413+
referentially transparent function).
410414
- In Rust, we don't have good tools for referentially transparent treatment
411415
of operations that involve side effects. If a statement produces or makes use
412416
of a side-effect, the context in which that statement is executed should use

0 commit comments

Comments
 (0)