From 7487eff3fc3b4a69a0a68842cda1c93d21bff7db Mon Sep 17 00:00:00 2001
From: JaredBorders <jaredborders@pm.me>
Date: Tue, 25 Jun 2024 14:26:18 -0400
Subject: [PATCH] =?UTF-8?q?=F0=9F=94=8E=20add=20PR:REVIEW=20tags?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 README.md                | 2 ++
 foundry.toml             | 7 +++++++
 package.json             | 1 +
 test/KSXVault.t.sol      | 6 ++++++
 test/Upgrade.t.sol       | 6 ++++++
 test/utils/Bootstrap.sol | 1 +
 6 files changed, 23 insertions(+)

diff --git a/README.md b/README.md
index e05f3b3..2f7657f 100644
--- a/README.md
+++ b/README.md
@@ -4,6 +4,8 @@
 [![Foundry][foundry-badge]][foundry] 
 [![License: GPL-3.0][license-badge]][license]
 
+
+/** @PR:REVIEW update following urls **/
 [gha]: https://github.com/Kwenta/foundry-scaffold/actions
 [gha-badge]: https://github.com/Kwenta/foundry-scaffold/actions/workflows/test.yml/badge.svg
 [foundry]: https://getfoundry.sh/
diff --git a/foundry.toml b/foundry.toml
index 7670d09..7803d10 100644
--- a/foundry.toml
+++ b/foundry.toml
@@ -10,6 +10,13 @@ optimizer_runs = 1_000_000
 [fmt]
 line_length = 80
 number_underscore = "thousands"
+/** @PR:REVIEW recommend adding:
+multiline_func_header = "all"
+sort_imports = true
+contract_new_lines = true
+override_spacing = false
+wrap_comments = true
+**/
 
 [rpc_endpoints]
 mainnet = "${MAINNET_RPC_URL}" 
diff --git a/package.json b/package.json
index 7b10517..697ee7c 100644
--- a/package.json
+++ b/package.json
@@ -11,6 +11,7 @@
     "coverage:generate-lcov": "forge coverage --fork-url $(grep OPTIMISM_GOERLI_RPC_URL .env | cut -d '=' -f2) --report lcov",
     "analysis:slither": "slither .",
     "gas-snapshot": "forge snapshot --fork-url $(grep OPTIMISM_GOERLI_RPC_URL .env | cut -d '=' -f2)",
+    /** @PR:REVIEW synthetix not used **/
     "decode-custom-error": "npx @usecannon/cli decode synthetix-perps-market"
   },
   "repository": {
diff --git a/test/KSXVault.t.sol b/test/KSXVault.t.sol
index a980305..bf8617a 100644
--- a/test/KSXVault.t.sol
+++ b/test/KSXVault.t.sol
@@ -5,6 +5,12 @@ import {Test} from "forge-std/Test.sol";
 import {KSXVault} from "../src/KSXVault.sol";
 import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 
+/** @PR:REVIEW no tests for vault **/
+/** @PR:REVIEW add pDAO test **/
+/** @PR:REVIEW add share name **/
+/** @PR:REVIEW add share symbol **/
+/** @PR:REVIEW add use bootstrap everywhere instead of `Test` **/
+
 contract KSXVaultTest is Test {
     function setUp() public {}
 }
diff --git a/test/Upgrade.t.sol b/test/Upgrade.t.sol
index 4add5f0..bb46281 100644
--- a/test/Upgrade.t.sol
+++ b/test/Upgrade.t.sol
@@ -7,10 +7,13 @@ import {MockVaultUpgrade} from "test/utils/mocks/MockVaultUpgrade.sol";
 
 contract UpgradeTest is Bootstrap {
     function setUp() public {
+        /** @PR:REVIEW unless there is a specific reason, avoid using fork tests **/
         initializeOptimism();
     }
 }
 
+/** @PR:REVIEW not entirely sure why you split up UpgradeTest/MockUpgrade/UpgradeVault; if no specific reason combine. otherwise thats fine **/
+
 contract MockUpgrade is UpgradeTest {
     MockVaultUpgrade mockVaultUpgrade;
 
@@ -22,6 +25,9 @@ contract MockUpgrade is UpgradeTest {
     }
 
     function test_upgrade() public {
+
+        /** @PR:REVIEW might as well fuzz the messages; never know what might be caught **/
+
         string memory message = "hi";
 
         bool success;
diff --git a/test/utils/Bootstrap.sol b/test/utils/Bootstrap.sol
index 7402cb7..2a65d79 100644
--- a/test/utils/Bootstrap.sol
+++ b/test/utils/Bootstrap.sol
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 pragma solidity 0.8.25;
 
+/** @PR:REVIEW remove any unused imports (except console bc it is helpful) **/
 import {console2} from "lib/forge-std/src/console2.sol";
 import {
     KSXVault