Skip to content

chore: Added a top-level Makefile in python-sdk to simplify running tests and linters #241

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.

Sign up for GitHub

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

Jump to bottom
Merged
SoulPancake merged 4 commits into from
Oct 25, 2025
Merged

chore: Added a top-level Makefile in python-sdk to simplify running tests and linters #241

SoulPancake merged 4 commits into from
Oct 25, 2025

Conversation

@Abishek-Newar
Copy link
Contributor

@Abishek-Newar Abishek-Newar commented Oct 25, 2025 •
edited by coderabbitai bot
Loading

Description

Added a standardized Makefile would make it much easier to test and lint the code locally with simple, consistent commands which would make it easy for contrubuters.

What problem is being solved?

How is it being solved?

What changes are made to solve it?

Added a Makefile to it

References

Closes #240

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

Summary by CodeRabbit

  • Chores
    • Added standardized development commands to streamline common project tasks including dependency management, testing with coverage reporting, code linting, and code formatting.

@Abishek-Newar Abishek-Newar requested a review from a team as a code owner October 25, 2025 13:37
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 25, 2025 •
edited
Loading

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

A Makefile was added at the project root defining seven automation targets for common development tasks: dependency synchronization via uv sync, test execution with coverage reporting, static linting and code formatting using ruff, a combined check target, and a help documentation target.

Changes

Cohort / File(s) Summary
New Makefile
Makefile		 Added seven phony make targets: sync (install/update dependencies), test (run pytest with coverage), lint (run ruff static analysis), fmt (format code with ruff), check (execute lint and test), doc (display help message)

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Straightforward addition of standard make targets with no complex logic or conditional branching

Possibly related issues

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues Check ✅ Passed The PR successfully implements all required objectives from issue #240. The Makefile includes all four mandated targets: sync (uv sync for dependencies), test (pytest with coverage reporting), lint (ruff checks), and fmt (ruff format). These targets directly address the issue's goal of providing standardized local development commands that mirror CI workflows without Docker. The PR also includes two additional targets (check for running both lint and test, and doc for help documentation) that enhance usability without conflicting with the stated requirements.
Out of Scope Changes Check ✅ Passed All changes in the PR are directly aligned with the objectives from issue #240. The Makefile targets (sync, test, lint, fmt) match the issue requirements exactly, and the additional targets (check and doc) serve the broader goal of simplifying contributor onboarding and ensuring parity between local and CI environments. No modifications to other files or unrelated functionality are present in the changeset.
Title Check ✅ Passed The PR title "chore: Added a top-level Makefile in python-sdk to simplify running tests and linters" directly and accurately describes the main change in the changeset. The summary confirms that a new Makefile was added with targets for common developer tasks (sync, test, lint, fmt, check, doc), and the objectives align with making it easier to run tests and linters locally. The title is concise, uses standard commit convention, and is specific enough that a teammate scanning git history would immediately understand the primary change without ambiguity.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Oct 25, 2025
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
Makefile (1)

25-31: doc target exceeds checkmake style guidance.

The doc target body contains 7 lines of output commands, exceeding checkmake's recommended maximum of 5 lines. While this is a style preference rather than a correctness issue, consider condensing the help output if verbosity becomes a concern, or document the exception.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 759afd0 and 1d55791.

📒 Files selected for processing (1)
  • Makefile (1 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile

[warning] 25-25: Target body for "doc" exceeds allowed length of 5 (7).

(maxbodylength)

[warning] 1-1: Missing required phony target "all"

(minphony)

[warning] 1-1: Missing required phony target "clean"

(minphony)

🔇 Additional comments (2)
Makefile (2)

10-11: Verify test target invocation syntax.

The test target correctly uses $(if $(TEST),$(TEST),test/) to conditionally run a specific test or default to the test/ directory. The coverage configuration and pytest arguments align with the PR objectives.

Confirm that openfga_sdk is the correct package name for coverage instrumentation by searching the repository structure.

4-5: Targets sync, lint, and fmt are well-formed.

These targets correctly delegate to uv for dependency management, linting, and formatting, matching the stated objectives.

Also applies to: 14-15, 18-19

SoulPancake
SoulPancake requested changes Oct 25, 2025
View reviewed changes
@Abishek-Newar @SoulPancake
Made suggested changes in MakeFile
37d7938 
Fixed a typo in MakeFile

Co-authored-by: Anurag Bandyopadhyay <[email protected]>
SoulPancake
SoulPancake reviewed Oct 25, 2025
View reviewed changes
Copy link
Member

@SoulPancake SoulPancake left a comment

Choose a reason for hiding this comment

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

Can you also add a check --fix command ?

@Abishek-Newar Abishek-Newar changed the title chore: Added a top-level Makefile in js-sdk to simplify running tests and linters chore: Added a top-level Makefile in python-sdk to simplify running tests and linters Oct 25, 2025
@Abishek-Newar
Copy link
Contributor Author

Abishek-Newar commented Oct 25, 2025

@SoulPancake , I have added the check --fix, is ther anything more that i can do

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.86%. Comparing base (759afd0) to head (7ebdcdb).

❌ Your project status has failed because the head coverage (70.86%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #241   +/-   ##
=======================================
  Coverage   70.86%   70.86%           
=======================================
  Files         135      135           
  Lines       10932    10932           
=======================================
  Hits         7747     7747           
  Misses       3185     3185

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

SoulPancake
SoulPancake approved these changes Oct 25, 2025
View reviewed changes
Copy link
Member

@SoulPancake SoulPancake left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM 🚀

@SoulPancake SoulPancake added this pull request to the merge queue Oct 25, 2025
Merged via the queue into openfga:main with commit 94e817a Oct 25, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@SoulPancake SoulPancake SoulPancake approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a top-level Makefile in python-sdk to simplify running tests and linters

3 participants

@Abishek-Newar @codecov-commenter @SoulPancake