Skip to content

Conversation

rguan72
Copy link
Contributor

@rguan72 rguan72 commented Aug 18, 2025

Description

It's a bit confusing because these pip install commands need to be run relative to one folder above the root onyx folder. I think we should give the commands relative to running them from the main onyx folder (so you'd run these right after clone)

How Has This Been Tested?

[Describe the tests you ran to verify your changes]

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

  • This PR should be backported (make sure to check that the backport attempt succeeds)
  • [Optional] Override Linear Check

@rguan72 rguan72 requested a review from a team as a code owner August 18, 2025 22:00
Copy link

vercel bot commented Aug 18, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
internal-search Ready Ready Preview Comment Aug 18, 2025 10:05pm

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR updates the pip install instructions in the CONTRIBUTING.md documentation to use relative paths from the project root directory. The change modifies four pip install commands (lines 106-109) by removing the onyx/ prefix from the file paths, changing them from onyx/backend/requirements/*.txt to backend/requirements/*.txt.

This documentation improvement standardizes the contribution guide to assume developers are working from the root project directory, which aligns with the typical workflow where contributors clone the repository and work from within it. The change makes the documentation more consistent, as other commands throughout the file (like Docker commands and directory navigation instructions) already assume the developer is positioned at the project root. This creates a more coherent and user-friendly setup experience for new contributors.

Confidence score: 5/5

  • This PR is safe to merge with minimal risk as it only updates documentation paths
  • Score reflects a simple documentation fix with no functional code changes that could cause runtime issues
  • No files require special attention as this is purely a documentation update

1 file reviewed, no comments

Edit Code Review Bot Settings | Greptile

@rguan72 rguan72 changed the title Update pip install instructions to be relative to root project directory docs(readme): Update pip install instructions to be relative to root project directory Aug 18, 2025
@rguan72 rguan72 changed the title docs(readme): Update pip install instructions to be relative to root project directory docs(readme): update pip install instructions to be relative to root project directory Aug 18, 2025
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

@evan-onyx evan-onyx added this pull request to the merge queue Aug 20, 2025
github-merge-queue bot pushed a commit that referenced this pull request Aug 20, 2025
Co-authored-by: Richard Guan <richardguan@Richards-MBP.lan>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 20, 2025
@evan-onyx evan-onyx added this pull request to the merge queue Aug 20, 2025
Merged via the queue into main with commit de7fe93 Aug 20, 2025
19 of 26 checks passed
@evan-onyx evan-onyx deleted the richard/update-pip-install-to-be-relative-to-project-dir branch August 20, 2025 03:50
AnkitTukatek pushed a commit to TukaTek/onyx that referenced this pull request Sep 23, 2025
Co-authored-by: Richard Guan <richardguan@Richards-MBP.lan>
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.

2 participants