diff --git a/.github/ISSUE_TEMPLATE/bug-report.md b/.github/ISSUE_TEMPLATE/bug-report.md new file mode 100644 index 0000000..7ee5091 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug-report.md @@ -0,0 +1,51 @@ +--- +name: Bug report +about: Create a report about a bug in Secant iOS Wallet. +title: '' +labels: 'bug' +assignees: '' + +--- + + + +### Describe the issue +Please provide a general summary of the issue you're experiencing + +### Can you reliably reproduce the issue? +#### If so, please list the steps to reproduce below: +1. +2. +3. + +### Expected behaviour +Tell us what should happen + +### Actual behaviour + errors +Tell us what happens instead including any noticable error output (any messages +displayed on-screen when e.g. a crash occurred) + + +- App Version: +- iOS Version: +- Device: (if applies) + +### Any extra information that might be useful in the debugging process. + + diff --git a/.github/ISSUE_TEMPLATE/feature-request.md b/.github/ISSUE_TEMPLATE/feature-request.md new file mode 100644 index 0000000..34d0f55 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature-request.md @@ -0,0 +1,22 @@ +--- +name: Feature request +about: Suggest an idea for the Secant wallet. +title: '' +labels: 'use case' +assignees: '' + +--- + +## Is your feature request related to a problem? Please describe. +A clear and concise description of what the problem is. Example: I'm always +frustrated when [...] + +## Describe the solution you'd like +A clear and concise description of what you want to happen. + +## Alternatives you've considered +A clear and concise description of any alternative solutions or features you've +considered. + +## Additional context +Add any other context or screenshots about the feature request here. diff --git a/.github/ISSUE_TEMPLATE/ux-report.md b/.github/ISSUE_TEMPLATE/ux-report.md new file mode 100644 index 0000000..55d9643 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/ux-report.md @@ -0,0 +1,18 @@ +--- +name: UX report +about: Was this wallet hard to use? It's not you, it's us. We want to hear about it. +title: 'UX: ' +labels: 'usability' +assignees: '' + +--- + + + +## What were you trying to do + +## What happened + diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..c2c9073 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,21 @@ +This code review checklist is intended to serve as a starting point for the author and reviewer, although it may not be appropriate for all types of changes (e.g. fixing a spelling typo in documentation). For more in-depth discussion of how we think about code review, please see [Code Review Guidelines](../blob/main/CODE_REVIEW_GUIDELINES.md). + +# Author + +- [ ] Self-review: Did you review your own code in GitHub's web interface? Code often looks different when reviewing the diff in a browser, making it easier to spot potential bugs. +- [ ] Automated tests: Did you add appropriate automated tests for any code changes? +- [ ] Code coverage: Did you check the code coverage report for the automated tests? Coverage can be generated by running `./gradlew check -PisCoverageEnabled=true; ./gradlew jacocoTestReport -PisCoverageEnabled=true` or by downloading the test report from the CI results. While we are not looking for perfect coverage, the tool can point out potential cases that have been missed. +- [ ] Documentation: Did you update Docs as appropiate? (E.g [README.md](../blob/main/README.md), [LICENSE.md](../blob/main/LICENSE.md), etc.) +- [ ] Run the app: Did you run the app and try the changes? +- [ ] Did you provide Screenshots of what the App looks like before and your changes? (only applicable to UI Changes) +- [ ] Rebase and squash: Did you pull in the latest changes from the main branch and squash your commits before assigning a reviewer? Having your code up to date and squashed will make it easier for others to review. Use best judgement when squashing commits, as some changes (such as refactoring) might be easier to review as a separate commit. + + +# Reviewer + +- [ ] Checklist review: Did you go through the code with the [Code Review Guidelines](../blob/main/CODE_REVIEW_GUIDELINES.md) checklist? +- [ ] Ad hoc review: Did you perform an ad hoc review? +- [ ] Automated tests: Did you review the automated tests? +- [ ] Manual tests: Did you review the manual tests? +- [ ] Documentation: Did you review Docs, [README.md](../blob/master/README.md), [LICENSE.md](../blob/master/LICENSE.md), and [Architecture.md](../blob/master/docs/Architecture.md) as appropriate? +- [ ] Run the app: Did you run the app and try the changes? While the CI server runs the app to look for build failures or crashes, humans running the app are more likely to notice unexpected log messages, UI inconsistencies, or bad output data. \ No newline at end of file diff --git a/CODE_REVIEW_GUIDELINES.md b/CODE_REVIEW_GUIDELINES.md new file mode 100644 index 0000000..0d5dc88 --- /dev/null +++ b/CODE_REVIEW_GUIDELINES.md @@ -0,0 +1,47 @@ +# Overall: + +- Does the contribution referece an existing Issue? +- Are the requirements well defined? + + +# Specification: + +- Are the requirements for the change well specified? Where are they documented? Bugfixes need a clear specification of the bug's cause and fix. (Small PRs might specify in a github ticket. Large changes may require stand-alone docs, or ZIPs.) +- Is there a documented test plan, which describes how to manually verify the change works on testnet prior to a release? + +# User Documentation: + +- What is the "changelog" entry for this change? (All changes should include this.) +- Are there any changes which require updates to user-facing documentation? If so, does the new documentation make sense? + +# Testing: + +- For non-minor PRs (up to the code reviewers and PR creator if this is needed), both the person who opened the PR and at least one of the code reviewers need to run a node with the updated code on testnet and again on mainnet to make sure nothing obvious breaks. +- Are there new tests that check all of the requirements? If it's a bugfix does it include new tests testing the bug triggering condition? (Do they fail before the fix and pass after the fix?) +- Do tests include edge cases, error conditions, and "negative case" tests to ensure the software is robust? Example: a function for verifying transaction signatures should include a bunch of tests for invalid signature cases. +- Do tests include all "logical test coverage" in addition to requirements-focused tests? Logical test coverage verifies behavior of the code that isn't obvious or implied by the requirements themselves. +- Have all of the automated tests been executed by CI? Read over the CI report to verify this. (Note: our CI system may not currently provide test results prior to review. We should change this! In the meantime, the reviewer should run the tests.) +- Is it clear that the test plan does actually test the new change? Does the test plan include any edge case / negative case / error conditions? + +# Code Review: +- Does the code structure make sense? Does it follow existing conventions / frameworks of existing code, or is it introducing new abstractions / structure? +- Is the code style consistent with the [Swift Style Guide](SWIFTLINT.md)? +- Does every change make sense? Make sure to ask questions, even or especially "basic" coding questions ("what does this mean in Swift?") and domain-newbie questions ("what is the priority system, and why does this line change the priority?"). + +In summary, here's a checklist that summarizes what reviewers should be looking for when doing a Code Review [1] + +- The code is well-designed. +- The functionality is good for the users of the code. +- Any UI changes are sensible and look good. +- Any parallel programming is done safely. +- The code isn’t more complex than it needs to be. +- The developer isn’t implementing things they might need in the future but don’t know they need now. +- Code has appropriate unit tests. +- Tests are well-designed. +- The developer used clear names for everything. +- Comments are clear and useful, and mostly explain why instead of what. +- Code is appropriately documented (generally in g3doc). +- The code conforms to our style guides. +- Make sure to review every line of code you’ve been asked to review, look at the context, make sure you’re improving code health, and compliment developers on good things that they do. + +[[1] What to look for in a code review](https://google.github.io/eng-practices/review/reviewer/looking-for.html) \ No newline at end of file diff --git a/SWIFTLINT.md b/SWIFTLINT.md new file mode 100644 index 0000000..0c1671c --- /dev/null +++ b/SWIFTLINT.md @@ -0,0 +1,138 @@ +# Swift Style Guide + +The SwiftLint configuration on this repo is based on the great work of the raywenderlich.com folks [The Official raywenderlich.com Swift Style Guide](https://github.com/raywenderlich/swift-style-guide). + +The purpose of this guide is to provide a base format for our Swift codebase. This +guide describes what are the rules enforced by our swift lint rules and should be +used when contributing or reviewing code. + +These guides use SwiftLint as a standard. You can learn more about SwiftLint by visiting its [official documentation page](https://github.com/realm/SwiftLint). + +## Table of Contents + +* [Installing SwiftLint](#installing-swiftlint) +* [Using the configuration file](#using-the-configuration-file) +* [Xcode settings](#xcode-settings) +* [Running SwiftLint](#running-swiftlint) +* [Handling rule exceptions](#handling-rule-exceptions) +* [Approved exceptions](#approved-exceptions) + * [Implicitly Unwrapped Optionals](#implicitly-unwrapped-optionals) + * [Force cast](#force-cast) + * [Force unwrapping](#force-unwrapping) + * [SwiftUI and multiple trailing closures](#swiftui-and-multiple-trailing-closures) + * [Open-source files](#open-source-files) +* [Other notes](#other-notes) + +## Installing SwiftLint + + +```bash +brew install swiftlint +``` + +If you are unable to use Homebrew, you may use one of the other methods described in the [SwiftLint documentation](https://github.com/realm/SwiftLint). + +**Do not** install SwiftLint as a CocoaPod in your project. + + +## Xcode settings + +You'll need to configure Xcode to remove trailing whitespace from all lines. This is not the default configuration. + +In Xcode's Preferences, select **Text Editing** ▸ **Editing** and check **Including whitespace-only lines**. + +![](screens/trailing-whitespace.png) + +## Handling rule exceptions + +Your sample project must compile without warnings — SwiftLint or otherwise. In general, you should change your code to eliminate all warnings where necessary. When it comes to SwiftLint, however, there will be times when this isn't possible. In these situations, you'll need to use in-line comments to temporarily disable rules. You can find appropriate syntax to do this in [the SwiftLint documentation](https://realm.github.io/SwiftLint/#disable-rules-in-code). + +You may only disable a rule if it is on the list of approved exceptions listed below. + +Prefer the form that disables a rule only for the next line: +``` +// swiftlint:disable:next implicitly_unwrapped_optional +var injectedValue: Data! +``` + +Or, similarly, for the previous line: +``` +var injectedValue: Data! +// swiftlint:disable:previous implicitly_unwrapped_optional +``` + +If there are several approved exceptions, group them together and disable the rule for that region. Be sure to enable the rule after that section. Do not leave a rule disabled throughout the source file. + +``` +// swiftlint:disable implicitly_unwrapped_optional +var injectedValue1: Data! +var injectedValue2: Data! +// swiftlint:enable implicitly_unwrapped_optional +``` + +If you must disable rules in your project, leave those in-line comments in the project for the benefit of your teammates in the editing pipeline. + +Finally, if you're not sure which rule is triggering a warning, you can find the rule name in parentheses at the end of message: + +![](screens/swiftlint-warning.png) + +## Approved exceptions + +There are certain common idioms that violate SwiftLint's strict checking. If you are unable to work around them — and you've already tried to find a better way to structure your code — you may disable rules as described in this section. + +If you find that you're struggling with rules other than those described below, please reach out to your Team Lead with your specific example. + +### Implicitly Unwrapped Optionals + +It is sometimes common, in lieu of using dependency injection, to declare a child view controller's properties as Implicitly Unwrapped Optionals (IUO). If you're unable to structure your project to avoid this, you may disable the `implicitly_unwrapped_optional` rule for those dependency declarations. With the advent of `@IBSegueAction`, this should be rare. + +### Force cast + +You may use force casting — and disable the `force_cast` rule — in the `UITableViewDataSource` and `UICollectionViewDataSource` methods that dequeue cells. + +### Force unwrapping + +You may use forced unwrapping — rule name `force_unwrap` — when returning a color from an asset catalog: + +``` +static var rwGreen: UIColor { + // swiftlint:disable:next force_unwrapping + UIColor(named: "rw-green")! +} +``` + +You may also use it in the same context as the force cast exception above, dequeuing cells in `UITableViewDataSource` and `UICollectionViewDataSource` methods. + +Although it's preferred that you model appropriately defensive code for our readers, you may use force unwrapping to access resources that you _know_ are included in the app bundle. + +Finally, you may use force unwrapping when constructing a `URL` from a hard-coded, and guaranteed valid, URL string. + +### SwiftUI and multiple trailing closures + +Idiomatic SwiftUI uses trailing closures to provide the view content for certain user interface elements. `Button` is a prime example; it has an initializer form that uses a closure to provide its `label`. It's common to write something like the following: + +``` +Button(action: { self.isPresented.toggle() }) { + Image(systemName: "plus") +} +``` + +This violates the rule that you should not use trailing closure syntax when a method accepts multiple closure parameters, so SwiftLint will flag it with a warning. + +You can work around this by extracting the `Button`'s action into a separate method. While this is frequently a better solution when the action requires several statements, it's an onerous requirement when the action is a single statement, as in the example above. + +In these cases, you're permitted to disable this rule **for the declaration of a SwiftUI view** only. The rule name is `multiple_closures_with_trailing_closure`. + +### Open-source files + +Occasionally, you'll find it necessary to include an unmodified open-source file in the sample project. It's a virtual certainty that these files won't comply with our style guide. Our practice has always been to leave these files in their original state. In this situation, you should disable SwiftLint for the entire file: + +``` +// swiftlint:disable all +``` + +## Other notes + +While SwiftLint goes a long way towards making your source code compliant with our style guide, it doesn't cover everything. For example, it won't catch or force you to correct the formatting for multi-condition `guard` statements. (See [Golden Path](https://github.com/raywenderlich/swift-style-guide#golden-path) for correct formatting.) + +This configuration has been tested against several dozen of our most recent tutorials. A couple of rules, such as the line length limit or the limit on the length of a function, may need tweaking to fit our style. If you find yourself butting heads with SwiftLint, please reach out to the iOS Team Lead with details. \ No newline at end of file diff --git a/co.electriccoin.swiftlint.yml b/co.electriccoin.swiftlint.yml new file mode 100644 index 0000000..afe37c4 --- /dev/null +++ b/co.electriccoin.swiftlint.yml @@ -0,0 +1,110 @@ +# This SwiftLint file is based on this great guidelines. +# https://github.com/raywenderlich/swift-style-guide + +excluded: + - ${PWD}/Carthage + - ${PWD}/Pods + - ${PWD}/DerivedData + - ${PWD}/xctemplates + +disabled_rules: + - discarded_notification_center_observer + - notification_center_detachment + - orphaned_doc_comment + - todo + - unused_capture_list + +opt_in_rules: + - array_init + - attributes + - closure_end_indentation + - closure_spacing + - collection_alignment + - colon # promote to error + - convenience_type + - discouraged_object_literal + - empty_collection_literal + - empty_count + - empty_string + - enum_case_associated_values_count + - fatal_error_message + - first_where + - force_unwrapping + - implicitly_unwrapped_optional + - indentation_width + - last_where + - legacy_random + - literal_expression_end_indentation + - multiline_arguments + - multiline_function_chains + - multiline_literal_brackets + - multiline_parameters + - multiline_parameters_brackets + - operator_usage_whitespace + - overridden_super_call + - pattern_matching_keywords + - prefer_self_type_over_type_of_self + - redundant_nil_coalescing + - redundant_type_annotation + - strict_fileprivate + - toggle_bool + # - trailing_closure # weird in SwiftUI + - unneeded_parentheses_in_closure_argument + - unused_import + - vertical_whitespace_closing_braces + - vertical_whitespace_opening_braces + - yoda_condition + + +custom_rules: + array_constructor: + name: "Array/Dictionary initializer" + regex: '[let,var] .+ = (\[.+\]\(\))' + capture_group: 1 + message: "Use explicit type annotation when initializing empty arrays and dictionaries" + severity: warning + + +attributes: + always_on_same_line: + - "@IBSegueAction" + - "@IBAction" + - "@NSManaged" + - "@objc" + +force_cast: warning +force_try: warning +function_body_length: + warning: 60 + +legacy_hashing: error + +identifier_name: + excluded: + - i + - id + - x + - y + - z + +indentation_width: + indentation_width: 2 + +line_length: + ignores_urls: true + ignores_function_declarations: true + ignores_comments: true + +multiline_arguments: + first_argument_location: next_line + only_enforce_after_first_closure_on_first_line: true + +private_over_fileprivate: + validate_extensions: true + +trailing_whitespace: + ignores_empty_lines: true + ignores_comments: true + +vertical_whitespace: + max_empty_lines: 2 diff --git a/screens/indentation.png b/screens/indentation.png new file mode 100644 index 0000000..11bef64 Binary files /dev/null and b/screens/indentation.png differ diff --git a/screens/project_settings.png b/screens/project_settings.png new file mode 100644 index 0000000..5e8515a Binary files /dev/null and b/screens/project_settings.png differ diff --git a/screens/swiftlint-warning.png b/screens/swiftlint-warning.png new file mode 100644 index 0000000..70938f4 Binary files /dev/null and b/screens/swiftlint-warning.png differ diff --git a/screens/trailing-whitespace.png b/screens/trailing-whitespace.png new file mode 100644 index 0000000..e1da4f8 Binary files /dev/null and b/screens/trailing-whitespace.png differ diff --git a/screens/xcode-jump-bar.png b/screens/xcode-jump-bar.png new file mode 100644 index 0000000..7e61bb1 Binary files /dev/null and b/screens/xcode-jump-bar.png differ diff --git a/zcash.swiftformat b/zcash.swiftformat deleted file mode 100644 index 2e89e4b..0000000 --- a/zcash.swiftformat +++ /dev/null @@ -1,8 +0,0 @@ -# options ---self remove # redundantSelf ---importgrouping testable-bottom # sortedImports ---commas always # trailingCommas ---trimwhitespace always # trailingSpace - -# rules ---rules redundantParens,redundantSelf,sortedImports,trailingCommas,trailingSpace