-
Notifications
You must be signed in to change notification settings - Fork 217
chore: migrate from Makefile to Just for build and development commands #271
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
Conversation
- Replaced Makefile with Justfile for improved command management. - Updated pre-commit hooks to use Just commands for linting and formatting. - Modified documentation to reflect the new Just commands for installation, building, and testing. - Removed Makefile and associated makefiles for a cleaner project structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 1287297 in 1 minute and 58 seconds. Click for details.
- Reviewed
948lines of code in19files - Skipped
0files when reviewing. - Skipped posting
12draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/build-rust.yml:38
- Draft comment:
Updated SOURCE_HASH command: replacing 'Makefile' and 'makefiles' with 'justfile' is correct and consistent with the migration. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it simply states that the update is correct and consistent with the migration. It does not provide any actionable feedback or suggestions for improvement.
2. .github/workflows/build-rust.yml:42
- Draft comment:
The new step to install 'just' using extractions/setup-just@v3 ensures that the just commands can execute without issues. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only states that a new step ensures something without providing any actionable feedback or suggestions. It doesn't align with the rules for good comments, which should provide specific suggestions or ask for confirmation on specific changes.
3. .github/workflows/rust-unit.yml:38
- Draft comment:
Adding the 'Install just' step and replacing make commands with 'just' (e.g. 'just check' and 'just build') makes the workflow consistent with the new build system. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it simply states that the changes make the workflow consistent with the new build system. It does not provide any actionable feedback or suggestions for improvement.
4. .github/workflows/rust.yml:10
- Draft comment:
Replaced Makefile and makefiles references with 'justfile' in the trigger paths; this aligns the CI workflows with the migration. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining a change that was made without providing any actionable feedback or suggestions. It does not align with the rules for useful comments.
5. .github/workflows/typescript-integration.yml:15
- Draft comment:
Workflow trigger paths now include 'justfile' instead of Makefile/makefiles, ensuring TS integration tests run on relevant changes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining a change in the workflow trigger paths. It does not provide a suggestion, ask for confirmation, or point out a potential issue. It simply states what has been changed.
6. .pre-commit-config.yaml:5
- Draft comment:
Hook entry updated to 'just fmt' for Rust formatting; this update maintains consistency with the migration from make. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining a change that was made. It doesn't provide any actionable feedback or suggestions for improvement. It doesn't align with the rules for good comments, as it doesn't ask for confirmation, suggest improvements, or identify potential issues.
7. CLAUDE.md:42
- Draft comment:
Documentation now uses 'just' commands (e.g., 'just test-integration', 'just build') instead of make; the instructions are clear and consistent. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, praising the use ofjustcommands overmake. It doesn't provide any actionable feedback or suggestions for improvement.
8. README.md:100
- Draft comment:
Local development instructions have been updated to use 'just' (e.g., 'just install', 'just build', 'just run'); this update ensures newcomers follow the new command structure. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, providing details about updates to local development instructions. It doesn't suggest any code changes or ask for specific confirmations.
9. crates/lib/src/metrics/README.md:23
- Draft comment:
Updated the metrics documentation to use 'just run-metrics'; ensure that the underlying justfile target matches the intended behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure that the underlying justfile target matches the intended behavior. This falls under asking the author to ensure something, which is against the rules.
10. justfile:1
- Draft comment:
The new justfile is comprehensive and well organized, covering build, test, service run, SDK management, docs, coverage, and release processes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the organization of the justfile without offering any specific insights or recommendations.
11. Makefile:1
- Draft comment:
Legacy Makefile has been removed along with its auxiliary makefiles, which is appropriate given the migration to just. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. makefiles/BUILD.makefile:1
- Draft comment:
Removal of BUILD.makefile (and similar auxiliary makefiles) is clean; ensure that all necessary targets are now fully covered in the justfile. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_mYryO0A2PxHyAH7M
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
pkxro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
amilz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this requires a new prereq for ppl using our commands, wdyt about adding something like this to readme under local dev?
Local development and testing use [Just](https://github.com/casey/just) as a build and development tool--make sure to install it before running any commands.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed d8d9d30 in 41 seconds. Click for details.
- Reviewed
14lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. README.md:91
- Draft comment:
Nice update adding Just as a prerequisite. For clarity, consider including a brief note on how to install Just (or its minimum version, if applicable) to help new developers get started. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_yKBA6XprOUN4GYOY
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Important
Migrate from Makefile to Justfile for command management, updating CI workflows, pre-commit hooks, and documentation accordingly.
MakefilewithJustfilefor build and development commands.Makefileand associatedmakefiles..github/workflows/build-rust.yml,.github/workflows/rust-unit.yml,.github/workflows/rust.yml, and.github/workflows/typescript-integration.ymlto usejustcommands instead ofmake..pre-commit-config.yamlto usejust fmtandjust format-ts-sdkfor formatting.README.md,CLAUDE.md, andcrates/lib/src/metrics/README.mdto reflect newjustcommands for installation, building, and testing.This description was created by
for d8d9d30. You can customize this summary. It will automatically update as commits are pushed.
📊 Unit Test Coverage
Unit Test Coverage: 80.8%
View Detailed Coverage Report