Repository navigation
Use lowercase names for cronic's variables, so that it does not change the caller's exported variables - #132
Conversation
…e the caller's exported variables
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe script renames option-state and temporary-file variables to lowercase Priority: ⬇️ Low Change: Bug fix Merge Risk: ⚪ Minimal · up to The variable rename is covered against the prior uppercase-variable collision; no actionable merge-blocking risk remains in these changes. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
… it assigns Environment variables are uppercase by convention, so lowercase names cannot overwrite a caller's exported variable (including `CRONIC_*` ones). The test now derives the variable names from `cronic` itself, and unsets the exported variables after the check.
cronic's variables with CRONIC_cronic's variables, so that it does not change the caller's exported variables
cronicassigned variables with common names such asDEBUG,OUT,ERR,TRACE, andRESULT. If the caller had exported a variable with one of those names,cronicchanged its value, and the wrapped command sawcronic's value rather than the caller's.This pull request gives all of
cronic's variables lowercase names that start withcronic_, matching the prefix that its functions already have. By convention, exported (environment) variables are uppercase, so lowercase names do not clash with them. It also adds a test that derives the names of the variables fromcronicitself and checks that the wrapped command sees the caller's exported value for each name in uppercase, both with and without theCRONIC_prefix.This is split out of #129, which relies on the prefixes.