Cleanup DApp - Remove Unused Stuff, Reorganize Rest #88

amessbee · 2024-11-12T10:05:39Z

As discussed in the Nov 4, 2024, Team-Devrel sync, here is a proposed cleanup of a few things before we add more contracts.

The changes in this pull request are:

.github/workflows/pr.yml

Updated build step name from "Build dapp-agoric-basics" to "Build dapp-orchestration-basics"
Changed logging steps and verification step names to be more descriptive

.gitignore

Added contract/bundles and new entries for e2e-testing files

contract/Makefile

Updated and removed unused targets

package.json and contract/package.json

Removed unused scripts.

Removed unused/duplicate scripts and updated corresponding paths.

Ref(s):

app starter #30

mujahidkay

Good stuff. And this is a good start. It cleans up the major, glaring unused stuff and I'm happy with this. Left a couple minor comments. We can do follow-up cleanups along the way when incorporating other contracts

mujahidkay · 2024-11-14T19:37:39Z

package.json

    "start:ui": "cd ui && yarn dev",
    "format": "yarn prettier --write .github contract ui",
    "lint:format": "yarn prettier --check .github contract ui",
    "lint": "yarn lint:format && yarn workspaces foreach --all run lint",
-    "lint:fix": "yarn format && yarn workspaces foreach --all run lint:fix",


I believe we need to keep this

mujahidkay · 2024-11-14T19:40:31Z

package.json

    "test": "yarn workspaces foreach --all run test",
-    "build": "yarn workspaces foreach --all run build",
-    "postinstall": "npx patch-package"


not sure if there are any repercussions for removing this. Will let @Jovonni confirm this.

@mujahidkay brought them back.

mujahidkay

Please capture the relevant GH issue in PR description before merging (and after @Jovonni 's signoff/approval)

dckc

various suggestions...

dckc · 2024-11-20T16:12:52Z

contract/package.json

-    "start:docker": "docker compose up -d",
-    "docker:logs": "docker compose logs --tail 200 -f",
-    "docker:bash": "docker compose exec agd bash",


Since this start:docker and docker:logs are our getting started docs, I suggest that yarn start:docker and the like should give a diagnostic about what to do instead. (Be sure to give a failing exit code.)

Or... if it's easy enough to just make docker:logs work (i.e. show the agd logs) then we should do that. (likewise docker:bash)

Good point.

start:docker now points to readme in multichain-testing to setup env

docker:logs now shows agd logs

docker:bash now opens agoriclocal-genesis-0 shell

dckc · 2024-11-20T16:13:17Z

contract/package.json

-    "docker:bash": "docker compose exec agd bash",
-    "docker:make": "docker compose exec agd make -C /workspace/contract",
-    "make:help": "make list",
-    "start": "make clean start",


likewise yarn start should give a clue about what to do instead.

yarn start now deploys contract on chain, i.e., make e2e
Updated the rest same as above.

dckc · 2024-11-20T16:14:27Z

ui/package.json

@@ -9,9 +9,7 @@
    "build": "tsc && NODE_OPTIONS=--max-old-space-size=4096 vite build",
    "lint": "yarn tsc && eslint . --ext ts,tsx --report-unused-disable-directives --max-warnings 0",
    "lint:fix": "yarn lint --fix",
-    "preview": "vite preview",
-    "test": "exit 0",


yarn test seems like it should stay

dckc · 2024-11-20T16:15:50Z

contract/Makefile


-PROVISION_POOL_ADDR=agoric1megzytg65cyrgzs6fvzxgrcqvwwl7ugpt62346
+ADDR := $(shell kubectl exec -i agoriclocal-genesis-0 -c validator -- sh -c '\
+    if ! agd keys show $(USERNAME) >/dev/null 2>&1; then \


Is there some reason we don't use a fixed mnemonic and address for user1?

Plan to work on this in a subsequent PR unless someone else gets to it before me.

dckc · 2024-11-20T16:21:43Z

contract/Makefile

@@ -163,12 +50,9 @@ fund: fund-provision-pool check-balance
 fund-osmo:
 	kubectl exec -i osmosislocal-genesis-0 -c validator -- osmosisd tx bank send faucet ${CLIENT_OSMO_ADDR} 9870000000uosmo --fees 1000uosmo -y;


I suggest factoring out EXEC_AGD and EXEC_OSMO like...

dapp-orchestration-basics/e2e-testing/Makefile

Line 10 in c4ea224

EXEC_AG0=kubectl exec -i agoriclocal-genesis-0 -c validator --

Then the k8s stuff doesn't get in the way when reading stuff like...

dapp-orchestration-basics/e2e-testing/Makefile

Lines 92 to 93 in c4ea224

provision-smart-wallet:

$(EXEC_AG0) agd tx swingset provision-one wallet $(ADDR) SMART_WALLET --from $(ADDR) -y -b block

dckc · 2024-11-20T16:22:29Z

contract/tools/explode-bundle.sh

tools is usually js code. Maybe scripts?

Unless there is a known purpose for this script, I am removing it.

dckc · 2024-11-20T16:23:48Z

contract/src/tools/debug.js

- * @param {string} name
- * @param {boolean | 'verbose'} enable
- */
-export const makeTracer = (name, enable = true) => {


Are we getting makeTracer from somewhere else?

If we're just not using it currently, I suggest keeping it around for occasional usage.

This a duplicate file - there is already another one in use.

dckc · 2024-11-20T16:24:25Z

contract/package.json

@@ -6,6 +6,7 @@
  "type": "module",
  "scripts": {
    "test": "ava",
+    "build:deployer": "rollup -c rollup.config.mjs",


Is this project still using rollup?

No, it does not but there some tests still using it. Maybe we can clean that later.

dckc · 2024-11-20T16:25:30Z

contract/scripts/init-orca.js

There's a reference to this file from deployment code that needs to be updated, I'm pretty sure.

ok... looks like it was fixed in a later commit.

dckc · 2024-11-20T16:27:22Z

contract/src/platform-goals/README.md

Are we keeping contract/src/platform-goals/start-contract.js? Then the README helps a little.

README is back - although it does not say anything useful AFAICT.

dckc

My comments have been addressed; thanks.

As I wasn't at the Nov 4 meeting, I don't think I'm in a good position to approve this.

amessbee added 3 commits November 12, 2024 14:42

chore: remove unused scripts from package.json

f6720a5

chore: remove unused startup files

4388e69

chore: clean contract/Makefile

eecdb19

amessbee force-pushed the ms/cleanup branch 11 times, most recently from 505ea34 to 83e9cf2 Compare November 13, 2024 04:01

amessbee added 5 commits November 13, 2024 11:11

chore: relocate unused utility script

15ff112

chore: update gitignore, delete duplicate copies of scripts

6411fe3

chore: relocate init script - update paths

7d2a6c9

chore: deleting (PROBABLY) unused files

1fcb5e0

chore: remove unused scripts

35e7d93

amessbee force-pushed the ms/cleanup branch 2 times, most recently from ae90c94 to 73bbd40 Compare November 13, 2024 06:17

chore: files needed for integration

f2e6eaf

amessbee force-pushed the ms/cleanup branch from 73bbd40 to f2e6eaf Compare November 13, 2024 06:33

amessbee marked this pull request as ready for review November 13, 2024 06:49

amessbee requested review from Jovonni and mujahidkay November 13, 2024 06:49

mujahidkay reviewed Nov 14, 2024

View reviewed changes

fixup! addition to package scripts

0443e6b

amessbee requested a review from mujahidkay November 15, 2024 04:13

mujahidkay approved these changes Nov 15, 2024

View reviewed changes

dckc reviewed Nov 20, 2024

View reviewed changes

amessbee added 2 commits November 21, 2024 18:05

chore: factor out EXEC_AGD and EXEC_OSMO

6dbc2d3

chore: update package.json

3b12483

amessbee requested a review from dckc November 22, 2024 04:52

dckc reviewed Nov 22, 2024

View reviewed changes

Jovonni approved these changes Dec 5, 2024

View reviewed changes

amessbee merged commit 0688d41 into main Dec 6, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup DApp - Remove Unused Stuff, Reorganize Rest #88

Cleanup DApp - Remove Unused Stuff, Reorganize Rest #88

amessbee commented Nov 12, 2024 •

edited

Loading

mujahidkay left a comment

mujahidkay Nov 14, 2024

mujahidkay Nov 14, 2024

amessbee Nov 15, 2024

mujahidkay left a comment

dckc left a comment

dckc Nov 20, 2024

amessbee Nov 22, 2024

dckc Nov 20, 2024

amessbee Nov 22, 2024

dckc Nov 20, 2024

amessbee Nov 22, 2024

dckc Nov 20, 2024

amessbee Nov 22, 2024

dckc Nov 20, 2024

amessbee Nov 22, 2024

dckc Nov 20, 2024

amessbee Nov 22, 2024

dckc Nov 20, 2024

amessbee Nov 22, 2024

dckc Nov 20, 2024

amessbee Nov 22, 2024

dckc Nov 20, 2024

dckc Nov 20, 2024

amessbee Nov 22, 2024

dckc left a comment

		@@ -163,12 +50,9 @@ fund: fund-provision-pool check-balance
		fund-osmo:
		kubectl exec -i osmosislocal-genesis-0 -c validator -- osmosisd tx bank send faucet ${CLIENT_OSMO_ADDR} 9870000000uosmo --fees 1000uosmo -y;

	provision-smart-wallet:
	$(EXEC_AG0) agd tx swingset provision-one wallet $(ADDR) SMART_WALLET --from $(ADDR) -y -b block

Cleanup DApp - Remove Unused Stuff, Reorganize Rest #88

Cleanup DApp - Remove Unused Stuff, Reorganize Rest #88

Conversation

amessbee commented Nov 12, 2024 • edited Loading

mujahidkay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mujahidkay left a comment

Choose a reason for hiding this comment

dckc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc left a comment

Choose a reason for hiding this comment

amessbee commented Nov 12, 2024 •

edited

Loading