Pull Request changes

added changelog file, docs folder.
changes to code review guidelines, lint rules, PR template

.github/CHANGELOG.md

@@ -0,0 +1,3 @@
+# Changelog
+
+- Added Code Review Guides, Changelog, pull request and issue templates, SwiftLint Rules

.github/pull_request_template.md

@@ -4,7 +4,7 @@ This code review checklist is intended to serve as a starting point for the auth
 <!-- NOTE: Do not modify these when initially opening the pull request.  This is a checklist template that you tick off AFTER the pull request is created. -->
 - [ ] 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.
+- [ ] Code coverage: Did you check the code coverage report for the automated tests?  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)
@@ -14,7 +14,7 @@ This code review checklist is intended to serve as a starting point for the auth
 # 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?
+- [ ] Ad hoc review: Did you perform an ad hoc review? _Did you perform an ad hoc review? In addition to a first pass using the code review guidelines, do a second pass using your best judgement and experience which may identify additional questions or comments. Research shows that code review is most effective when done in multiple passes, where reviewers look for different things through each pass._
 - [ ] 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?

CODE_REVIEW_GUIDELINES.md

@@ -12,11 +12,12 @@
 # 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?
+- Are there any changes which require updates to user-facing documentation? If so, does the new documentation make sense? (documentation should be placed in [docs/](/docs) folder of the repo)
 
 # 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.
+- For non-minor PRs (up to the code reviewers and PR creator if this is needed), we require authors to perform a thorough self-review and self-test of the resulting code base, including use cases that might not be visible affected by the introduced changes. Reviewers are not expected to run the changes locally, but are definitely encouraged to do so at their best judgement. 
+- When introducing modifications that affect the UI, screenshots might be provided in a BEFORE/AFTER fashion to speed up UI/UX requirement validation. 
 - 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.
@@ -35,7 +36,7 @@ In summary, here's a checklist that summarizes what reviewers should be looking
 - 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.
+- The developer isn’t implementing things they might need in the future but don’t know they need now or aren't using now.
 - Code has appropriate unit tests.
 - Tests are well-designed.
 - The developer used clear names for everything.

co.electriccoin.swiftlint.yml

@@ -1,20 +1,20 @@
-# This SwiftLint file is based on this great guidelines. 
+# This SwiftLint file is based on this great guideline. 
 # 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:
+  - nesting # allow for types to be nested, common pattern in Swift
+  - multiple_closures_with_trailing_closure 
+  - generic_type_name # allow for arbitrarily long generic type names
   - array_init
   - attributes
   - closure_end_indentation
@@ -36,16 +36,19 @@ opt_in_rules:
   - legacy_random
   - literal_expression_end_indentation
   - multiline_arguments
+  - multiline_argument_brackets
   - multiline_function_chains
   - multiline_literal_brackets
   - multiline_parameters
   - multiline_parameters_brackets
+  - no_space_in_method_call
   - operator_usage_whitespace
   - overridden_super_call
   - pattern_matching_keywords
   - prefer_self_type_over_type_of_self
   - redundant_nil_coalescing
   - redundant_type_annotation
+  - return_arrow_whitespace
   - strict_fileprivate
   - toggle_bool
   # - trailing_closure # weird in SwiftUI
@@ -53,6 +56,7 @@ opt_in_rules:
   - unused_import
   - vertical_whitespace_closing_braces
   - vertical_whitespace_opening_braces
+  - weak_delegate
   - yoda_condition
 
 
@@ -64,6 +68,30 @@ custom_rules:
     message: "Use explicit type annotation when initializing empty arrays and dictionaries"
     severity: warning
 
+  string_concatenation:
+    included: ".*\.swift"
+    excluded: ".*Test\.swift"
+    name: "String Concatenation"
+    regex: '' \+ "|" \+ |\+= "'
+    message: "Please use string interpolation instead of concatenation"
+    severity: error
+
+  print_function_usage:
+    included: ".*\.swift"
+    excluded: ".*Test\.swift"
+    name: "Swift print() or debugPrint() should not be used in App Code"
+    regex: "print\(|debugPrint\("
+    message: "The Swift print() or debugPrint() functions should not be used."
+    severity: warning
+
+  nslog_function_usage:
+    included: ".*\.swift"
+    excluded: ".*Test\.swift"
+    name: "Swift NSLog() should not be used in App Code"
+    regex: "NSLog\("
+    message: "The swift NSLog function should not be used."
+    severity: error
+
 
 attributes:
   always_on_same_line:
@@ -75,7 +103,7 @@ attributes:
 force_cast: warning
 force_try: warning
 function_body_length:
-    warning: 60
+    warning: 150
 
 legacy_hashing: error
 
@@ -91,6 +119,7 @@ indentation_width:
   indentation_width: 2
 
 line_length:
+  warning: 600
   ignores_urls: true
   ignores_function_declarations: true
   ignores_comments: true

docs/README.md

@@ -0,0 +1,4 @@
+# Documentation
+
+Here you'll find documentation
+TBD