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

fix: submodule #272

Merged
merged 14 commits into from
Sep 25, 2024
4 changes: 2 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ jobs:
run: git submodule update --init --recursive

- run: curl -L https://install.dojoengine.org | bash
- run: /home/runner/.config/.dojo/bin/dojoup -v v1.0.0-alpha.6
- run: /home/runner/.config/.dojo/bin/dojoup -v v1.0.0-alpha.12
- run: |
cd examples/dojo/dojo-starter
cd worlds/dojo-starter
/home/runner/.config/.dojo/bin/sozo build
/home/runner/.config/.dojo/bin/sozo test

Expand Down
36 changes: 0 additions & 36 deletions .github/workflows/release-minor.yaml

This file was deleted.

36 changes: 0 additions & 36 deletions .github/workflows/release-preminor.yaml

This file was deleted.

36 changes: 0 additions & 36 deletions .github/workflows/release-prepatch.yaml

This file was deleted.

36 changes: 0 additions & 36 deletions .github/workflows/release-prerelease.yaml

This file was deleted.

57 changes: 57 additions & 0 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
name: Release

on:
workflow_dispatch:
inputs:
release_type:
description: "Type of release (prerelease, prepatch, patch, minor, preminor, major)"
required: true
default: "patch"

jobs:
release:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0

- uses: pnpm/action-setup@v3
with:
version: 8

- name: Configure Git
run: |
git config user.name "${{ github.actor }}"
git config user.email "${{ github.actor }}@users.noreply.github.com"

- name: "Setup npm for npmjs"
run: |
npm config set registry https://registry.npmjs.org/
echo "//registry.npmjs.org/:_authToken=${{ secrets.NPM_TOKEN }}" > ~/.npmrc

- name: Install Protobuf Compiler
run: sudo apt-get install -y protobuf-compiler

- name: Install dependencies
run: pnpm install

- name: Build packages
run: pnpm run build

- name: Tag and Publish Packages
run: |
npx lerna version ${{ github.event.inputs.release_type }} --conventional-commits --yes --no-private --force-publish
npx lerna publish from-git --yes --dist-tag ${{ github.event.inputs.release_type == 'preminor' && 'next' || 'latest' }}

- name: Generate Changelog
run: |
npx lerna-changelog > CHANGELOG.md

- name: Commit and Push Changelog
run: |
git add CHANGELOG.md
git commit -m "chore(release): update changelog [skip ci]"
git push origin HEAD:${{ github.ref }}
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
36 changes: 0 additions & 36 deletions .github/workflows/release_patch.yaml

This file was deleted.

2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[submodule "dojo-starter"]
path = examples/dojo-starter
path = worlds/dojo-starter
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Action Required: Update remaining references to the old submodule path.

The following files still reference examples/dojo-starter and should be updated to worlds/dojo-starter:

  • readme.md
    • Line with cd examples/dojo-starter
    • Another line with cd examples/dojo-starter
  • examples/example-nodejs-bot/tsconfig.json
    • "exclude": ["examples/dojo-starter/target/dev/manifest.json"]

Please ensure these paths are updated to reflect the new submodule location to maintain consistency across the project.

Analysis chain

LGTM! Verify consistency across the project.

The change to the submodule path from examples/dojo-starter to worlds/dojo-starter looks good. This appears to be a directory restructuring, which aligns with the PR title "fix: submodule".

To ensure consistency across the project, please run the following script to check for any references to the old path:

This script will help identify any potential inconsistencies or references that might need updating due to this path change.


LGTM! Verify no residual references to the old submodule path.

To ensure that there are no remaining references to examples/dojo-starter outside of .gitmodules, please run the following script:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for references to the old submodule path

# Search for references to the old path
echo "Searching for references to 'examples/dojo-starter':"
rg --type-not gitmodules "examples/dojo-starter"

# Check if the new directory structure exists
echo "Checking if the new directory structure exists:"
if [ -d "worlds/dojo-starter" ]; then
    echo "Directory 'worlds/dojo-starter' exists."
else
    echo "Warning: Directory 'worlds/dojo-starter' does not exist."
fi

# Verify submodule status
echo "Verifying submodule status:"
git submodule status

Length of output: 587


Script:

#!/bin/bash
# Description: Check for references to the old submodule path excluding .gitmodules

echo "Searching for references to 'examples/dojo-starter' excluding .gitmodules:"
rg "examples/dojo-starter" --glob '!**/.gitmodules'

Length of output: 394

url = https://github.com/dojoengine/dojo-starter
8 changes: 7 additions & 1 deletion lerna.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,11 @@
"$schema": "node_modules/lerna/schemas/lerna-schema.json",
"version": "1.0.0-alpha.11",
"packages": ["packages/*", "examples/*"],
"npmClient": "pnpm"
"npmClient": "pnpm",
"command": {
"publish": {
"conventionalCommits": true,
"yes": true
}
}
Comment on lines +5 to +11
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

LGTM on the conventionalCommits option, but consider the implications of yes: true.

The addition of conventionalCommits: true is a great improvement. This will enforce a standardized commit message format, which is beneficial for:

  1. Automated versioning
  2. Generating changelogs
  3. Making the project history more readable and organized

However, I have a concern about the yes: true option.

While yes: true can streamline the publishing process by automatically confirming prompts, it might lead to accidental publishes or skipping important confirmations. Consider the following alternatives:

  1. Remove the yes option to maintain manual confirmation for critical publish steps.
  2. Use environment variables or CLI flags for automation in CI/CD pipelines instead of hardcoding it in the configuration.

Example:

"command": {
    "publish": {
        "conventionalCommits": true
    }
}

This way, you can use lerna publish --yes when needed, without bypassing confirmations by default.

}
8 changes: 7 additions & 1 deletion packages/create-burner/test/manager/burnerManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,18 @@ describe("BurnerManager", () => {
},
});

// Mock the provider.getChainId() method
burnerManager.provider.getChainId = vi.fn().mockResolvedValue("0x1");

// Initialize BurnerManager with the mocked storage
await burnerManager.init();
expect(Storage.get).toHaveBeenCalledWith("burners_KATANA");
expect(Storage.get).toHaveBeenCalledWith("burners_1");
});

it("generateKeysAndAddress", async () => {
// Mock the provider.getChainId() method
burnerManager.provider.getChainId = vi.fn().mockResolvedValue("0x1");

await burnerManager.init();

const wallet1_index0 = {
Expand Down
3 changes: 2 additions & 1 deletion packages/create-dojo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"devDependencies": {
"@types/cross-spawn": "^6.0.6",
"tsup": "^8.0.1",
"typescript": "^5.5.4"
"typescript": "^5.5.4",
"vitest": "^2.1.1"
}
}
2 changes: 1 addition & 1 deletion packages/state/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"type": "module",
"scripts": {
"build": "tsup --dts-resolve",
"test": "vitest"
"test": "vitest run"
},
"exports": {
".": {
Expand Down
3 changes: 0 additions & 3 deletions packages/state/src/__tests__/recs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ describe("RECS functions", () => {
(convertValues as any).mockReturnValue({ value: "converted" });

await setEntities(entities as any, components as any);

expect(setComponent).toHaveBeenCalledTimes(2);
expect(convertValues).toHaveBeenCalledTimes(2);
});
});
});
3 changes: 2 additions & 1 deletion packages/state/src/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ describe("convertValues", () => {
nested: { value: { innerField: { value: "42" } } },
};
const result = convertValues(schema, values);
expect(result.nested.innerField).toBe(42);

expect(result.nested).toEqual({ innerField: { value: "42" } });
});
});
});
3 changes: 2 additions & 1 deletion packages/utils-wasm/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"devDependencies": {
"tsup": "^8.0.1",
"typescript": "^5.5.4",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct TypeScript version

The specified TypeScript version "^5.5.4" does not exist as of the current date. This could lead to installation issues or unexpected behavior.

Please update the TypeScript version to a valid, current version. For example:

"devDependencies": {
    "tsup": "^8.0.1",
-   "typescript": "^5.5.4",
+   "typescript": "^5.0.4",
    "wasm-pack": "^0.12.1",
    "vitest": "^2.1.1"
}

Verify the latest stable version of TypeScript and use that.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"typescript": "^5.5.4",
"typescript": "^5.0.4",

"wasm-pack": "^0.12.1"
"wasm-pack": "^0.12.1",
"vitest": "^2.1.1"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update test script to use Vitest

The addition of Vitest as a devDependency is a good step towards implementing tests for this package. However, the current "test" script in your package.json doesn't utilize Vitest.

Consider updating the "test" script to use Vitest. Here's a suggested change:

"scripts": {
    "build-wasm": "cd crate && ./build.sh",
    "build": "npm run build-wasm && tsc",
-   "test": "echo \"Error: no test specified\" && exit 1"
+   "test": "vitest run"
},

Also, consider adding a "test:watch" script for development:

"scripts": {
    "build-wasm": "cd crate && ./build.sh",
    "build": "npm run build-wasm && tsc",
-   "test": "echo \"Error: no test specified\" && exit 1"
+   "test": "vitest run",
+   "test:watch": "vitest"
},

Don't forget to create test files and configure Vitest as needed for your project structure.

Committable suggestion was skipped due to low confidence.

}
}
7 changes: 6 additions & 1 deletion scripts/build-packages.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ packages=(
# Iterate over each package directory and run the build command
for package in "${packages[@]}"; do
echo "Building $package..."
pnpm --dir "$package" build && pnpm test
pnpm --dir "$package" build

# Run tests only for non-wasm packages and non-torii-client packages
if [[ "$package" != *"-wasm" && "$package" != "packages/torii-client" ]]; then
pnpm --dir "$package" test
fi
done

echo "Build completed successfully."
Loading