-
-
Notifications
You must be signed in to change notification settings - Fork 293
Move ds_version and ds_update_available to main.sh
#1966
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
`run_script 'ds-version'` was failing on fresh install because the `.scripts` folder wasn't there yet. Moved the function to `main.sh` to fix that. Did the same for `ds_update_available`.
Reviewer's GuideMigrated the ds_version and ds_update_available helper functions into main.sh to ensure they are available before the .scripts folder exists, updated all invocations to call these functions directly, and removed the now-obsolete external script files. Class diagram for refactored helper functions in main.shclassDiagram
class main.sh {
+ds_version() string
+ds_update_available() bool
}
%% The helper functions are now part of main.sh and are no longer in separate script files.
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @CLHatch - I've reviewed your changes - here's some feedback:
- Consider caching the results of ds_update_available and ds_version to avoid repeated git fetch calls during a single run.
- You could extract the pushd/popd+error‐handling pattern into a small helper function to reduce duplication and improve readability.
- Make sure to handle cases where there’s no upstream branch set, since git rev-parse '@{u}' will error out in those scenarios.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider caching the results of ds_update_available and ds_version to avoid repeated git fetch calls during a single run.
- You could extract the pushd/popd+error‐handling pattern into a small helper function to reduce duplication and improve readability.
- Make sure to handle cases where there’s no upstream branch set, since git rev-parse '@{u}' will error out in those scenarios.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
run_script 'ds-version'was failing on fresh install because the.scriptsfolder wasn't there yet. Moved the function tomain.shto fix that. Did the same fords_update_available.Pull request
Purpose
Describe the problem or feature in addition to a link to the issues.
Approach
How does this change address the problem?
Open Questions and Pre-Merge TODOs
Check all boxes as they are completed
Learning
Describe the research stage
Links to blog posts, patterns, libraries or addons used to solve this problem
Requirements
Check all boxes as they are completed
Summary by Sourcery
Move ds_version and ds_update_available functions into main.sh, remove their standalone scripts, and update invocations to call the functions directly.
Bug Fixes:
Enhancements: