-
Notifications
You must be signed in to change notification settings - Fork 241
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
feat: Implement automated styling on .R
files
#3408
Conversation
Some roxygen errors persist. Facing errors while trying to install RTM package |
echo "No changes made to: $file" | ||
else | ||
echo "Changes made to: $file. Re-adding to staging area." | ||
git add "$file" |
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.
Styling can change Roxygen output as well, so may need to re-roxygenize the relevant package dir here too. Not sure offhand if in this context it makes more sense to do that via make
or by directly invoking Roxygen.
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.
This will be easier to discuss, review, and get merged if you add the new scripts in one PR by itself and then do the initial styling of existing files in several separate PRs -- then we don't have to review >1200 files at once and can avoid creating merge conflicts with files touched by already-open PRs.
That said, from discussion to date in #3280 it's possible a pre-commit hook won't be the favored approach, so probably best to establish that before investing too many changes to this PR.
Signed-off-by: Abhinav Pandey <[email protected]>
@infotroph Closing this PR for now. I will open a seperate PR with documentations of all changes about the script |
Optimize R Formatting Script to Target Only Staged
.R
FilesFixes #3280 (Partially)
After reviewing discussion from #3280 and #3407 , I think one should take their time reviewing this. I will finish a review on each aspect of PEcAn to confirm if this script keeps the systems intact as they were previously.
Priority: Medium
Status: Ready for Review
Type: Enhancement
Description
I have optimized the existing R formatting script (
format.sh
) to improve its performance by targeting only the.R
files that are staged for commit in the Git repository.Key Changes:
Motivation and Context
This optimization aligns with best practices for Git hooks by minimizing the impact on commit performance, thereby encouraging team members to maintain code quality without frustration.
This PR addresses where the formatting script was identified as a bottleneck in the commit process.
Review Time Estimate
Types of changes
Checklist:
Additional Notes:
Documentation Update: While this change primarily affects the internal scripting for pre-commit hooks, I WILL updated the
CHANGELOG.md
to reflect the performance improvements if requested. If a more detailed documentation update is required, please let me know, and I can provide further details.Compatibility: This optimization is backward-compatible and should not affect existing workflows other than improving performance.