mirror of
https://github.com/apache/superset.git
synced 2026-05-22 08:15:36 +00:00
Copilot flagged two stragglers on editors.md where the previous file-by-file conversion stopped halfway. Sweeping for the same pattern across the active content tree found 76 bare relative internal links total — 14 in this PR's already-modified files (Copilot's two plus twelve more) and 62 in unchanged files. Why the build doesn't catch this ───────────────────────────────── `onBrokenLinks: 'throw'` (set in this PR) only validates *file-based* markdown references — links whose URL ends in `.md` / `.mdx`. Those go through Docusaurus's file resolver, which can prove the target exists. Bare relative URL paths like `[Foo](../foo)` skip that resolver entirely; Docusaurus emits them as raw hrefs. The browser then resolves them against the *current* page URL, and for trailing-slash routes that almost always lands in the wrong directory. Page navigates client-side and 404s. The linkinator job in CI *can* catch these, but it's `continue-on-error: true` so findings are advisory. What this commit does ────────────────────── 1. Fix all 76 bare relative internal links across the active docs tree by appending `.md` to each one (preserving anchors / query strings). All 76 targets resolved to real files; no link targets changed, only the form of the reference. 2. Fix the component-page generator. 54 of the 76 bare links lived in two auto-generated index files (`components/ui/index.mdx` and `components/design-system/index.mdx`). The next regeneration would have undone the manual fixes without this. The two emission sites in `generate-superset-components.mjs` now emit `.md`-suffixed links; comment at the call site explains why. 3. Add `docs/scripts/lint-docs-links.mjs` — fast source-level linter that scans `.md`/`.mdx` files under the active content trees (skipping `versioned_docs/` snapshots) and fails if it finds any markdown link whose URL starts with `./` or `../` and does not end in `.md`/`.mdx`. Excludes asset paths (.png, .json, etc.) and ignores fenced code blocks. Wired up as `yarn lint:docs-links`. 4. Add a `Lint docs links` step to `superset-docs-verify.yml`, running before the build step so PRs that introduce the pattern fail in seconds rather than at build-time / not at all. Blocking, not advisory — exactly the gap linkinator's `continue-on-error` leaves open. Verified ──────── - `yarn lint:docs-links` exits 0 on the cleaned tree - Re-introducing one bare link makes the linter report the exact file:line with the offending URL, exit code 1 - All 76 originally-flagged targets resolved to real `.md` / `.mdx` files; only the form of the reference changed
7.1 KiB
7.1 KiB
title, sidebar_position
| title | sidebar_position |
|---|---|
| Submitting Pull Requests | 3 |
Submitting Pull Requests
Learn how to create and submit high-quality pull requests to Apache Superset.
Before You Start
Prerequisites
- Development environment is set up
- You've forked and cloned the repository
- You've read the contributing overview
- You've found or created an issue to work on
PR Readiness Checklist
- Code follows coding guidelines
- Tests are passing locally
- Linting passes (
pre-commit run --all-files) - Documentation is updated if needed
Creating Your Pull Request
1. Create a Feature Branch
# Update your fork
git fetch upstream
git checkout master
git merge upstream/master
git push origin master
# Create feature branch
git checkout -b feature/your-feature-name
2. Make Your Changes
# Make changes
edit files...
# Run tests
pytest tests/unit_tests/
cd superset-frontend && npm run test
# Run linting
pre-commit run --all-files
# Commit with conventional format
git add .
git commit -m "feat(dashboard): add new filter component"
3. PR Title Format
Follow Conventional Commits:
type(scope): description
Types:
feat: New featurefix: Bug fixdocs: Documentation onlystyle: Code style (formatting, semicolons, etc.)refactor: Code refactoringperf: Performance improvementtest: Adding testschore: Maintenance tasksci: CI/CD changesbuild: Build system changesrevert: Reverting changes
Scopes:
dashboard: Dashboard functionalitysqllab: SQL Lab featuresexplore: Chart explorerchart: Visualization componentsapi: REST API endpointsdb: Database connectionssecurity: Security featuresconfig: Configuration
Examples:
feat(sqllab): add query cost estimation
fix(dashboard): resolve filter cascading issue
docs(api): update REST endpoint documentation
refactor(explore): simplify chart controls logic
perf(dashboard): optimize chart loading
4. PR Description Template
Use the template from .github/PULL_REQUEST_TEMPLATE.md:
### SUMMARY
Brief description of changes and motivation.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
[Required for UI changes]
### TESTING INSTRUCTIONS
1. Step-by-step instructions
2. How to verify the fix/feature
3. Any specific test scenarios
### ADDITIONAL INFORMATION
- [ ] Has associated issue: #12345
- [ ] Required feature flags:
- [ ] API changes:
- [ ] DB migration required:
### CHECKLIST
- [ ] CI checks pass
- [ ] Tests added/updated
- [ ] Documentation updated
- [ ] PR title follows conventions
5. Submit the PR
# Push to your fork
git push origin feature/your-feature-name
# Create PR via GitHub CLI
gh pr create --title "feat(sqllab): add query cost estimation" \
--body-file .github/PULL_REQUEST_TEMPLATE.md
# Or use the GitHub web interface
PR Best Practices
Keep PRs Focused
- One feature/fix per PR
- Break large changes into smaller PRs
- Separate refactoring from feature changes
Write Good Commit Messages
# Good
git commit -m "fix(dashboard): prevent duplicate API calls when filters change"
# Bad
git commit -m "fix bug"
git commit -m "updates"
Include Tests
# Backend test example
def test_new_feature():
"""Test that new feature works correctly."""
result = new_feature_function()
assert result == expected_value
// Frontend test example
test('renders new component', () => {
const { getByText } = render(<NewComponent />);
expect(getByText('Expected Text')).toBeInTheDocument();
});
Add Screenshots for UI Changes
### Before

### After

Update Documentation
- Update relevant docs in
/docsdirectory - Add docstrings to new functions/classes
- Update UPDATING.md for breaking changes
CI Checks
Required Checks
All PRs must pass:
Python Tests- Backend unit/integration testsFrontend Tests- JavaScript/TypeScript testsLinting- Code style checksType Checking- MyPy and TypeScriptLicense Check- Apache license headersDocumentation Build- Docs compile successfully
Common CI Failures
Python Test Failures
# Run locally to debug
pytest tests/unit_tests/ -v
pytest tests/integration_tests/ -v
Frontend Test Failures
cd superset-frontend
npm run test -- --coverage
Linting Failures
# Auto-fix many issues
pre-commit run --all-files
# Manual fixes may be needed for:
# - MyPy type errors
# - Complex ESLint issues
# - License headers
Responding to Reviews
Address Feedback Promptly
# Make requested changes
edit files...
# Add commits (don't amend during review)
git add .
git commit -m "fix: address review feedback"
git push origin feature/your-feature-name
Request Re-review
- Click "Re-request review" after addressing feedback
- Comment on resolved discussions
- Thank reviewers for their time
Handling Conflicts
# Update your branch
git fetch upstream
git rebase upstream/master
# Resolve conflicts
edit conflicted files...
git add .
git rebase --continue
# Force push (only to your feature branch!)
git push --force-with-lease origin feature/your-feature-name
After Merge
Clean Up
# Delete local branch
git checkout master
git branch -d feature/your-feature-name
# Delete remote branch
git push origin --delete feature/your-feature-name
# Update your fork
git fetch upstream
git merge upstream/master
git push origin master
Follow Up
- Monitor for any issues reported
- Help with documentation if needed
- Consider related improvements
Tips for Success
Do
- ✅ Keep PRs small and focused
- ✅ Write descriptive PR titles and descriptions
- ✅ Include tests for new functionality
- ✅ Respond to feedback constructively
- ✅ Update documentation
- ✅ Be patient with the review process
Don't
- ❌ Submit PRs with failing tests
- ❌ Include unrelated changes
- ❌ Force push to master
- ❌ Ignore CI failures
- ❌ Skip the PR template
Getting Help
- Slack: Ask in #development or #beginners
- GitHub: Tag @apache/superset-committers for attention
- Mailing List: dev@superset.apache.org