Skip to content

Commit d542f4b

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

File tree

1 file changed

+52
-45
lines changed

1 file changed

+52
-45
lines changed

CONTRIBUTING.md

Lines changed: 52 additions & 45 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

@@ -41,7 +41,7 @@ behavior as documented in the code of conduct.
4141
> If you want to ask a question, please ensure that you have read the available
4242
> documentation. Documentation is published to the [Zallet Book](https://zcash.github.io/wallet).
4343
44-
Before you ask a question, it is best to search for existing [Issues](/issues)
44+
Before you ask a question, it is best to search for existing [Issues](https://github.com/zcash/wallet/issues)
4545
that might help you. In case you have found a suitable issue and still need
4646
clarification, you can write your question in this issue. It is also advisable
4747
to search the internet for answers first.
@@ -53,7 +53,7 @@ recommend the following:
5353
There are no bad questions, only insufficiently documented answers. If you're
5454
able to find an answer and it wasn't already in the docs, consider opening a
5555
pull request to add it to the documentation!
56-
- You can also open an [Issue](/issues/new). If you do so:
56+
- You can also open an [Issue](https://github.com/zcash/wallet/issues/new). If you do so:
5757
- Provide as much context as you can about what you're running into.
5858
- Provide project and platform versions depending on what seems relevant.
5959

@@ -74,7 +74,7 @@ it is desirable for users to use the latest released version. Detailed
7474
change logs are available in the `CHANGELOG.md` file.
7575

7676
Please note that the wallet in this workspace is under continuous development
77-
and new SemVer major-version releases are frequent. Users of this application,
77+
and new SemVer major-version releases are frequent. Users of this application
7878
should expect a corresponding maintenance burden. The `CHANGELOG.md` file is
7979
vital to understanding these changes. Under normal circumstances, proposed
8080
changes will be considered for application against the last two major release
@@ -96,10 +96,10 @@ possible.
9696
documented preconditions for an operation.
9797
- To see if other users have experienced (and potentially already solved) the
9898
same issue you are having, check if there is not already a bug report
99-
existing for your bug or error in the [bug tracker](issues?q=label%3Abug).
99+
existing for your bug or error in the [bug tracker](https://github.com/zcash/wallet/issues?q=label%3Abug).
100100
- Also make sure to search the internet to see if users outside of the GitHub
101101
community have discussed the issue. You can also ask about your problem in
102-
the [Zcash R&D Discord](https://discordapp.com/channels/809218587167293450/876655911790321684).
102+
the [Zcash R&D Discord](https://discord.com/channels/809218587167293450/876655911790321684).
103103
- Collect information about the problem:
104104
- OS, Platform and Version (Windows, Linux, macOS, x86, ARM)
105105
- Version of the compiler, runtime environment, etc. depending on what seems
@@ -120,7 +120,7 @@ possible.
120120
We use GitHub issues to track bugs and errors. If you run into an issue with
121121
the project:
122122

123-
- Open an [Issue](/issues/new). (Since we can't be sure at this point whether
123+
- Open an [Issue](https://github.com/zcash/wallet/issues/new). (Since we can't be sure at this point whether
124124
the issue describes a bug or not, we ask you not to label the issue.)
125125
- Explain the behavior you would expect and the actual behavior.
126126
- Please provide as much context as possible and describe the **reproduction
@@ -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
@@ -159,7 +159,7 @@ community to understand your suggestion and find related suggestions.
159159

160160
- Read the documentation of the latest version of the appropriate crate to find
161161
out if the functionality is already provided, potentially under a feature flag.
162-
- Perform a [search](/issues) to see if the enhancement has already been
162+
- Perform a [search](https://github.com/zcash/wallet/issues) to see if the enhancement has already been
163163
suggested. If it has, add a comment to the existing issue instead of opening
164164
a new one.
165165
- Find out whether your idea fits with the scope and aims of the project. It's
@@ -176,7 +176,7 @@ community to understand your suggestion and find related suggestions.
176176

177177
#### How Do I Submit a Good Enhancement Suggestion?
178178

179-
Enhancement suggestions are tracked as [GitHub issues](/issues).
179+
Enhancement suggestions are tracked as [GitHub issues](https://github.com/zcash/wallet/issues).
180180

181181
- Use a **clear and descriptive title** for the issue to identify the
182182
suggestion. The relevant library crate, if known, should be indicated by prefixing
@@ -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,40 @@ 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 MUST pass `cargo clippy --all-targets -- -D warnings` (using the
269+
pinned MSRV toolchain). Additionally, PRs MUST NOT introduce new warnings from
270+
`cargo +beta clippy --tests --all-features --all-targets`. Preexisting beta
271+
clippy warnings need not be resolved, but new ones introduced by a PR will
272+
block merging. The first case described above for work-in-progress commits is
273+
excepted from these requirements.
274+
275+
#### Pull Requests
280276

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

283303
It is acceptable and desirable to use a rebase-based workflow within the
284304
context of a single pull request in order to produce a clean commit history.
@@ -339,25 +359,12 @@ To get around this GitHub UI limitation, the general process we follow is:
339359

340360
If a PR author is non-responsive to review comments, the crate maintainers may
341361
take over make the necessary changes to the PR ourselves. For PRs created from
342-
user forks we can generally do this in the same PR. PRs from anonther
362+
user forks we can generally do this in the same PR. PRs from another
343363
organization's forks usually do not allow changes from maintainers (due to
344364
missing cross-organization permissions); in this case (or if a user's PR has
345365
"allow maintainers to edit" disabled), we may close the PR and open a new PR
346366
containing the commits from the original.
347367

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-
361368
### Coding Style
362369

363370
The Zallet authors hold our software to a high standard of quality. The list of
@@ -387,9 +394,8 @@ implications, including but not limited to the following:
387394
state space are used.
388395
- Use custom `enum`s with semantically relevant variants instead of boolean
389396
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.
397+
- Make data types immutable unless there is a strong reason to believe that
398+
values will need to be modified in-place for performance reasons.
393399
- Take care when introducing and/or using structured enum variants, because
394400
Rust does not provide adequate language features for making such values
395401
immutable or ensuring safe construction. Instead of creating structured or
@@ -405,8 +411,9 @@ This means:
405411
- Write referentially transparent functions. A referentially transparent
406412
function is one that, given a particular input, always returns the same
407413
output.
408-
- Avoid mutation whenever possible. If it's strictly necessary, use mutable
409-
variables only in the narrowest possible scope.
414+
- Avoid mutation. If it's necessary for performance, use mutable variables only
415+
in the narrowest possible scope (e.g. within the internal scope of a
416+
referentially transparent function).
410417
- In Rust, we don't have good tools for referentially transparent treatment
411418
of operations that involve side effects. If a statement produces or makes use
412419
of a side-effect, the context in which that statement is executed should use

0 commit comments

Comments
 (0)