-
-
Notifications
You must be signed in to change notification settings - Fork 16.3k
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
Update check_git_status()
to run under ROOT
working directory
#5441
Update check_git_status()
to run under ROOT
working directory
#5441
Conversation
…erforming git ops
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.
👋 Hello @MrinalJain17, thank you for submitting a 🚀 PR! To allow your work to be integrated as seamlessly as possible, we advise you to:
- ✅ Verify your PR is up-to-date with upstream/master. If your PR is behind upstream/master an automatic GitHub actions rebase may be attempted by including the /rebase command in a comment body, or by running the following code, replacing 'feature' with the name of your local branch:
git remote add upstream https://github.com/ultralytics/yolov5.git
git fetch upstream
git checkout feature # <----- replace 'feature' with local branch name
git merge upstream/master
git push -u origin -f
- ✅ Verify all Continuous Integration (CI) checks are passing.
- ✅ Reduce changes to the absolute minimum required for your bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is." -Bruce Lee
@glenn-jocher This PR fixes the issue raised in #5440 . Let me know if you have any thoughts on this. |
@MrinalJain17 this is a nice solution! I like the context manager approach. My main question here is would it be possible to pass a directory argument to the context manager to generalize it's capability? i.e. something like: with ChangeWorkingDirectory(dir=ROOT):
# do stuff |
@glenn-jocher That makes sense. Updated the PR accordingly. I didn't set a default value within the context manager to avoid any unintended change in the working directory. The user has to explicitly specify the directory to which it should switch. |
check_git_status()
to run under ROOT
working directory
@MrinalJain17 PR is merged. Thank you for your contributions to YOLOv5 🚀 and Vision AI ⭐ |
…tralytics#5441) * Updating check_git_status() to switch to the repository root before performing git ops * More pythonic * Linting * Remove redundant Path() call * Generalizing the context manager * Fixing error in context manager * Cleanup * Change to decorator Co-authored-by: Glenn Jocher <[email protected]>
Fixes #5440
🛠️ PR Summary
Made with ❤️ by Ultralytics Actions
🌟 Summary
New context manager for changing working directory in code sections.
📊 Key Changes
WorkingDirectory
as a context manager to change the current working directory within a code block.@WorkingDirectory(ROOT)
decorator has been applied to thecheck_git_status
function.🎯 Purpose & Impact