-
Notifications
You must be signed in to change notification settings - Fork 14
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
1056 foxx test script fix #1064
base: devel
Are you sure you want to change the base?
Conversation
…st cases if specified
…ion where needed :-
Reviewer's Guide by SourceryThis PR fixes bash script behavior by improving environment variable handling and script path resolution. The changes make the scripts more robust by properly handling unset variables and using the correct method to determine script locations. Class diagram for updated bash script variablesclassDiagram
class TestFoxxScript {
-e : removed
-u : removed
+uf : added
SCRIPT : updated
DATAFED_DATABASE_PASSWORD : updated
FOXX_MAJOR_API_VERSION : updated
}
class DependencyInstallFunctionsScript {
SCRIPT : updated
}
note for TestFoxxScript "Improved handling of unset variables and script path resolution"
note for DependencyInstallFunctionsScript "Updated script path resolution method"
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 @JoshuaSBrown - I've reviewed your changes - here's some feedback:
Overall Comments:
- The comment about removing -u is incorrect since -u is now included in the set options. Please update the comment to reflect the current behavior.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…-part-of-ci 1057 trigger running foxx tests as part of ci
…-part-of-ci 1057 trigger running foxx tests as part of ci
1055 fix foxx setup script
#1056
Summary by Sourcery
Fix the Foxx test script to handle unset environment variables and improve script reliability by using BASH_SOURCE for script path determination.
Bug Fixes:
Enhancements: