Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
^\.Rproj\.user$
.*~
^\.circleci/config\.yml$
.github
94 changes: 94 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
## Code Review Guidelines

### R Package Standards
- This is a standard R package - all changes must maintain CRAN-compatible structure
- NAMESPACE is auto-generated by roxygen2 - never edit it directly
- DESCRIPTION must be kept valid with proper versioning
- All exported functions require complete roxygen2 documentation

### Code Style Requirements
- **Naming conventions**: Use `dotted.case`, `camelCase`, or `PascalCase` consistently
- Parameters/variables: `dotted.case` (e.g., `response.format`, `text.variables`, `api.key`)
- Functions: `camelCase` or `PascalCase` or `.camelCase` (e.g., `checkInputs`, `MakeOutput`, or `.nestedHelper`)
- Use `PascalCase` for exported functions (e.g., `AddFormulaBars`, `RegressionTable`)
- Use `camelCase` for internal/helper functions (e.g., `checkInputs`)
- Use `.camelCase` for internal nested functions
e.g., within an internal function
```
addOneToAll <- function(x) {
.helpAdd <- function(y) {
y + 1
}
vapply(x, .helpAdd, numeric(1L)
}
```
- **Object name length**: Maximum 50 characters
- **Assignment**: Always use `<-` for assignment, never `=`
- **Pipes**: Use native pipe `|>` for chaining operations
- **NULL coalescing**: Use `%||%` operator for NULL defaults
- **Indentation**: 4 spaces
- **Line length**: Maximum 120 characters
- **No trailing whitespace**
- **Consistent quotes**: Always use double quotes `"` for strings. It is ok for quotes within strings to be single quotes `'` but the parent string must use double quotes.
- **Prefer full words over abbreviations**: e.g., `response.format` over `resp.fmt`
- **Function names**: Use descriptive names that clearly indicate purpose: e.g. `createSummaryTableForRegressionResults()` rather than `make()`
- **Braces**: Always use braces `{}` for functions, `if`, `else if` and `else` blocks. Except for the case of a single line statements. In particular, use Kernighan & Ritchie style braces.
- e.g. The following is acceptable:
```
y <- if (x > 0) x else -x
w <- if (condition) {
z <- computeSomething(x)
foo(z)
} else {
z <- computeSomethingElse(x)
bar(z)
}
```
- **Redundant return**: Avoid using `return()` unless exiting early from a function. The last evaluated expression is returned by default.

### Function Design Patterns
- **Error handling**: Use `flipU::StopForUserError()` for user-facing errors with clear, actionable messages
- **Return values**: Functions used for validation/side effects should explicitly return `NULL`
- **Internal functions**: Internal or helper functions should not be exported; use `@noRd` in roxygen comments
- **File organization**: Group related functions into logical files (e.g., check.inputs.R, tidy.inputs.R)
- **Importing functions**: Use `@importFrom` in roxygen comments for all external package functions used and avoid usage of `:::`.
- **Type checking**: Don't use `sapply`. Use `vapply` with a specified return type for type safety.
- **Formatting numbers for printing**: Use `flipFormat::FormatAsReal()` and `flipFormat::FormatAsPercent()` instead of base R formatting functions
- **Calculations**: Use the following functions where possible:
- `verbs::Average` instead of `mean`
- `verbs::Sum` instead of `sum`
- `verbs::Min` instead of `min`
- `verbs::Max` instead of `max`
- `verbs::Variance` instead of `var`
- `verbs::StandardDeviation` instead of `sd`

### Documentation Requirements
- All exported functions must have complete roxygen2 documentation
- Required tags: `@param` (for each parameter), `@return`, `@export`
- Include `@importFrom` for all external package functions used
- Optional but recommended: `@details`, `@examples`

### Testing Requirements
- Use testthat v3 framework (edition 3)
- Test files must be in `tests/testthat/` with `test-` prefix
- Include snapshot tests where appropriate (`_snaps/` directory)

### Security & Best Practices
- **Never log or expose API keys or secrets**

## PR Review Focus Areas
1. **Breaking changes**: Flag any changes to exported function signatures
2. **Documentation**: Ensure all new/modified exports have complete roxygen2 docs
3. **Testing**: Verify new functionality has corresponding tests
4. **Style consistency**: Check adherence to naming convetions, `<-`, `|>` patterns
5. **Error messages**: Ensure user-facing errors are clear and helpful
6. **Line length**: Flag lines exceeding 120 characters
7. **Object names**: Ensure names don't exceed 50 characters

## Common Issues to Flag
- Using `=` instead of `<-` for assignment
- Missing or incomplete roxygen2 documentation
- Direct API key exposure or logging
- Inconsistent naming conventions (mixing dotted.case/camelCase inappropriately)
- New dependencies without justification
- Error messages that don't guide users to solutions
119 changes: 119 additions & 0 deletions .github/workflows/check-copilot-instructions.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
name: Check Copilot Instructions Sync

on:
pull_request:
branches: '*'

jobs:
check-copilot-instructions:
runs-on: ubuntu-latest

steps:
- name: Checkout current repository
uses: actions/checkout@v4

- name: Download copilot-instructions.md from flipU
id: download-source
run: |
echo "Downloading copilot-instructions.md from flipU..."

# Try multiple possible paths for the copilot instructions
POSSIBLE_PATHS=(
".github/copilot-instructions.md"
"copilot-instructions.md"
".github/copilot_instructions.md"
"docs/copilot-instructions.md"
)

DOWNLOADED=false
for path in "${POSSIBLE_PATHS[@]}"; do
echo "Trying path: $path"
if curl -s -H "Accept: application/vnd.github.v3.raw" \
-o /tmp/source-copilot-instructions.md \
"https://api.github.com/repos/Displayr/flipU/contents/$path" 2>/dev/null; then

# Check if we got actual content (not an error response)
if [ -f /tmp/source-copilot-instructions.md ] && [ -s /tmp/source-copilot-instructions.md ]; then
# Verify it's not a JSON error response
if ! grep -q '"message".*"Not Found"' /tmp/source-copilot-instructions.md; then
echo "✅ Successfully downloaded from: $path"
echo "source_path=$path" >> $GITHUB_OUTPUT
DOWNLOADED=true
break
fi
fi
fi
done

if [ "$DOWNLOADED" = false ]; then
echo "❌ Could not find copilot-instructions.md in flipU repo"
echo "This may indicate the file doesn't exist or has been moved."
echo "Please check the flipU repository structure."
exit 1
fi

- name: Check if local copilot-instructions.md exists
id: check-local
run: |
if [ ! -f .github/copilot-instructions.md ]; then
echo "❌ No copilot-instructions.md found in this repository"
echo "Expected location: .github/copilot-instructions.md"
echo "exists=false" >> $GITHUB_OUTPUT
exit 1
else
echo "✅ Found local copilot-instructions.md"
echo "exists=true" >> $GITHUB_OUTPUT
fi

- name: Compare files
run: |
echo "Comparing copilot-instructions.md files..."
echo "Source: flipU/${{ steps.download-source.outputs.source_path }}"
echo "Local: .github/copilot-instructions.md"
echo ""

# Compare the files
if diff -u /tmp/source-copilot-instructions.md .github/copilot-instructions.md > /tmp/diff_output; then
echo "✅ Files match perfectly!"
echo "Your copilot-instructions.md is up to date with flipU."
else
echo "❌ Files do not match!"
echo ""
echo "Differences found:"
echo "=================="
cat /tmp/diff_output
echo "=================="
echo ""
echo "To fix this issue:"
echo "1. Update your .github/copilot-instructions.md to match the source"
echo "2. Or run the shell script in flipU to do this for you."
echo " # Run these commands locally from the root of your repository"
echo " > curl -O -L https://raw.githubusercontent.com/Displayr/flipU/master/.github/update-copilot-instructions.sh"
echo " > bash update-copilot-instructions.sh"
echo " > git add .github/copilot-instructions.md"
echo " > git commit -m 'Update copilot-instructions.md to match flipU'"
echo " > git push"
echo "Source file location: flipU/${{ steps.download-source.outputs.source_path }}"
exit 1
fi

- name: Verify file integrity
run: |
echo "Verifying file integrity..."

# Check that the local file is not empty
if [ ! -s .github/copilot-instructions.md ]; then
echo "❌ Local copilot-instructions.md is empty"
exit 1
fi

# Check that it contains expected content patterns
if ! grep -q -i "copilot\|instruction\|coding\|standard" .github/copilot-instructions.md; then
echo "⚠️ Warning: File doesn't seem to contain typical copilot instruction content"
echo "Please verify the file contains the expected coding standards and instructions"
else
echo "✅ File appears to contain valid copilot instructions"
fi

echo "File size: $(wc -c < .github/copilot-instructions.md) bytes"
echo "Line count: $(wc -l < .github/copilot-instructions.md) lines"
Loading