Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: base for agent app #41

Merged
merged 9 commits into from
Sep 13, 2024
Merged

feat: base for agent app #41

merged 9 commits into from
Sep 13, 2024

Conversation

0xyaco
Copy link
Collaborator

@0xyaco 0xyaco commented Sep 13, 2024

🤖 Linear

Closes GRT-167

Description

Set up the basic configuration for the main agent app.

Copy link

linear bot commented Sep 13, 2024

GRT-167 Set up agent app

Copy link
Collaborator

@jahabeebs jahabeebs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thoughts on adding a ReadMe/license? or you think the ones in the base repo are enough? Maybe at least adding some instructions on running in the base repo ReadMe would be good

};

process.on("unhandledRejection", (reason, p) => {
console.error(`Unhandled Rejection at: \n${inspect(p, undefined, 100)}, \nreason: ${reason}`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional that we're logging with console rather than logger?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

{
"name": "agent",
"version": "1.0.0",
"description": "",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some other things we could add here would be license: MIT and private: true (though we should add these on our other packages as well to avoid accidental publishing)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the private property, need to ask for The Graph team about licensing.

@0xyaco
Copy link
Collaborator Author

0xyaco commented Sep 13, 2024

@jahabeebs totally! Docs, readme, licensing and stuff like that is going to be tackled by the end of the project. But overall, yeah, given the complexity of the project, docs must be super detailed and complete so they are a must.

0xkenj1
0xkenj1 previously approved these changes Sep 13, 2024
Copy link
Collaborator

@0xkenj1 0xkenj1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after addressing @jahabeebs comments :)

Base automatically changed from refactor/automated-dispute-imports to dev September 13, 2024 15:02
@0xyaco 0xyaco dismissed 0xkenj1’s stale review September 13, 2024 15:02

The base branch was changed.

@0xyaco 0xyaco requested review from jahabeebs and 0xkenj1 September 13, 2024 15:28
0xkenj1
0xkenj1 previously approved these changes Sep 13, 2024
0xnigir1
0xnigir1 previously approved these changes Sep 13, 2024
Copy link
Collaborator

@0xnigir1 0xnigir1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, left some suggestions (not critical) but i'm not sure if the paths I specified are correct

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you need a dev script for running it with hot reload? tsx is the one we used in chainhub

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I'll see how the development goes with this particular app. We'll probably end up needing it during some debugging sessions and such.

"type": "module",
"private": "true",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"private": "true",
"types": "./dist/index.d.ts",
"files": [
"dist/*",
"package.json",
"!**/*.tsbuildinfo"
],
"private": "true",

@@ -4,6 +4,7 @@
"description": "",
"main": "./dist/index.js",
"type": "module",
"private": "true",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"private": "true",
"types": "./dist/index.d.ts",
"files": [
"dist/*",
"package.json",
"!**/*.tsbuildinfo"
],
"private": "true",

@@ -4,6 +4,7 @@
"description": "",
"main": "./dist/index.js",
"type": "module",
"private": "true",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"private": "true",
"types": "./dist/index.d.ts",
"files": [
"dist/*",
"package.json",
"!**/*.tsbuildinfo"
],
"private": "true",

@0xyaco 0xyaco dismissed stale reviews from 0xnigir1 and 0xkenj1 via 7c8979a September 13, 2024 16:20
@0xyaco 0xyaco merged commit 2d1de10 into dev Sep 13, 2024
5 checks passed
@0xyaco 0xyaco deleted the feat/agent-app branch September 13, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants