Skip to content

chore(skills): simplify sl-commit and sl-submit-diff skill docs - #264

Merged
flora131 merged 3 commits into
mainfrom
flora131/feature/update-sl-commands
Feb 23, 2026
Merged

chore(skills): simplify sl-commit and sl-submit-diff skill docs#264
flora131 merged 3 commits into
mainfrom
flora131/feature/update-sl-commands

Conversation

@flora131

@flora131 flora131 commented Feb 23, 2026

Copy link
Copy Markdown
Collaborator

Streamlines Sapling skill documentation by removing unused commands, duplicate content, and focusing on the recommended Meta workflow. This reduces cognitive load and ensures coding agents use the most appropriate commands.

Summary

This PR cleans up the Sapling skill documentation to focus on the essential, commonly-used commands and removes outdated or Meta-internal workflow references. Changes are applied consistently across all three agent configurations (Claude Code, GitHub Copilot, OpenCode).

Changes

sl-commit skill

Removed unused commands:

  • sl bookmark - Not needed for basic commit workflow
  • sl smartlog -l 5 - Advanced feature, not essential
  • sl absorb - Advanced feature removed from command reference

Simplified commands:

  • sl diff --statsl diff (simpler, more commonly used)

Removed duplicate content:

  • Entire "Conventional Commits Format" section (already documented elsewhere in the codebase)
  • Smartlog and Absorb documentation from "Key Sapling Differences" section

sl-submit-diff skill

Standardized on draft workflow:

  • Updated all jf submit commands to jf submit --draft
  • Emphasizes DRAFT mode as the default submission method

Removed Meta-internal references:

  • Removed arc diff mentions (Meta-internal tool not available in open source)
  • Updated description to only reference jf submit (Meta workflow)

Simplified command reference:

  • Removed sl ssl from commands list
  • Removed advanced operations from Common Operations table:
    • View diff status
    • Check sync status
    • Get diff ID

Removed outdated section:

  • Entire "Prerequisites" section (.arcconfig, ~/.arcrc, fbcodereview extension requirements)

Impact

  • Better agent UX: Agents see only the essential commands they need
  • Consistency: All three agent configs (.claude, .github, .opencode) now have identical, streamlined skill docs
  • Focus: Documentation now focuses on the recommended Meta workflow with jf submit --draft

Files Changed

  • .claude/skills/sl-commit/SKILL.md
  • .claude/skills/sl-submit-diff/SKILL.md
  • .github/skills/sl-commit/SKILL.md
  • .github/skills/sl-submit-diff/SKILL.md
  • .opencode/skills/sl-commit/SKILL.md
  • .opencode/skills/sl-submit-diff/SKILL.md

Remove unused Sapling commands (bookmark, smartlog, absorb) and
duplicate conventional commits section from sl-commit. Simplify
sl-submit-diff to use only jf submit with --draft mode by default
and remove arc diff references and prerequisites section.

Assistant-model: Claude Code
@flora131
flora131 marked this pull request as ready for review February 23, 2026 04:20
Copilot AI review requested due to automatic review settings February 23, 2026 04:20
@claude

claude Bot commented Feb 23, 2026

Copy link
Copy Markdown

PR Review: chore(skills): simplify sl-commit and sl-submit-diff skill docs

Overview

This PR simplifies the Sapling skill documentation by removing unused commands and focusing on jf submit with draft mode as the default. The changes are applied consistently across all three config directories (.claude, .github, .opencode).

Files Changed: 6 | +18 / -78 (net reduction of 60 lines)


✅ What's Good

  1. Consistent changes across all skill directories - The same changes are properly applied to .claude/, .github/, and .opencode/ skills.

  2. Focused scope - Removing arc diff references and focusing on jf submit simplifies the documentation for the target use case (Meta workflows).

  3. Safer default - Using --draft mode by default is a safer approach for code review workflows, preventing accidental premature submissions.

  4. Reduced cognitive load - Removing rarely-used commands (sl bookmark, sl smartlog, sl absorb) makes the documentation more approachable.


⚠️ Suggestions for Improvement

1. Orphaned "Types:" Section (sl-commit/SKILL.md)

Location: Lines 45-47 in the resulting file

The "Conventional Commits Format" section was removed, but the "Types:" list remains without its header context. This leaves users without understanding what these "types" refer to.

Suggestion: Either:

  • Add a brief header like ## Conventional Commit Types before line 47, OR
  • Keep the minimal format explanation: Use conventional commit format: <type>: <description>

2. Double Blank Line Formatting

Location: Lines 44-46 in sl-commit/SKILL.md

After removing sl absorb, there's a double blank line before "Types:". Minor formatting cleanup would be helpful.

3. Missing Option for Non-Draft Submission (sl-submit-diff)

The documentation now only shows --draft usage. Consider adding a note about how to submit for immediate review when needed:

> **Note:** To submit without draft mode (ready for review immediately), use `jf submit` without the `--draft` flag.

4. Prerequisites Removal May Confuse New Users

The removed prerequisites section mentioned .arcconfig, ~/.arcrc, and fbcodereview. While this was for arc diff, users new to jf submit may still need setup guidance. Consider adding a brief note or link to jf submit setup docs if available.


🔒 Security

No security concerns - documentation-only changes.

🧪 Test Coverage

N/A - These are markdown documentation files.

⚡ Performance

N/A - No runtime impact.


Summary

This is a solid documentation cleanup PR. The changes improve focus and reduce noise. I'd suggest addressing the orphaned "Types:" section context and the minor formatting issue before merging, but these are not blockers.

Recommendation: ✅ Approve with minor suggestions

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies the Sapling skill documentation by removing unused commands and duplicate content from the sl-commit and sl-submit-diff skill files across three directories (.opencode, .github, .claude).

Changes:

  • Removed references to arc diff and open-source Phabricator support, focusing exclusively on jf submit
  • Changed all jf submit commands to use --draft flag by default
  • Removed unused Sapling commands (sl bookmark, sl smartlog -l 5, sl absorb) from sl-commit docs
  • Removed duplicate "Conventional Commits Format" section from sl-commit docs
  • Removed Prerequisites section from sl-submit-diff docs

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
.opencode/skills/sl-submit-diff/SKILL.md Updated to use only jf submit --draft, removed arc diff references and Prerequisites section
.opencode/skills/sl-commit/SKILL.md Removed bookmark/smartlog/absorb commands and Conventional Commits Format section
.github/skills/sl-submit-diff/SKILL.md Same changes as .opencode version for consistency
.github/skills/sl-commit/SKILL.md Same changes as .opencode version for consistency
.claude/skills/sl-submit-diff/SKILL.md Same changes as .opencode version for consistency
.claude/skills/sl-commit/SKILL.md Same changes as .opencode version for consistency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .opencode/skills/sl-commit/SKILL.md Outdated
Comment on lines 46 to 48

**Types:**

Copilot AI Feb 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The section header "Types:" appears orphaned after removing the "Conventional Commits Format" section. This creates a documentation structure issue where types are listed without the format context that explains how to use them. Consider either:

  1. Adding a brief header like "## Conventional Commit Types" before this line
  2. Re-introducing a simplified format explanation before the types list
  3. Moving this types list under the "What This Skill Does" section with appropriate context

The conventional commit format pattern (<type>[optional scope]: <description>) is still referenced in line 22 ("creates a commit message using conventional commit format") but the actual format is no longer documented in the file.

Suggested change
**Types:**
## Conventional Commit Format
Commit messages use the pattern: `<type>[optional scope]: <description>`.
### Conventional Commit Types

Copilot uses AI. Check for mistakes.
Comment thread .github/skills/sl-commit/SKILL.md Outdated
Comment on lines 46 to 48

**Types:**

Copilot AI Feb 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The section header "Types:" appears orphaned after removing the "Conventional Commits Format" section. This creates a documentation structure issue where types are listed without the format context that explains how to use them. Consider either:

  1. Adding a brief header like "## Conventional Commit Types" before this line
  2. Re-introducing a simplified format explanation before the types list
  3. Moving this types list under the "What This Skill Does" section with appropriate context

The conventional commit format pattern (<type>[optional scope]: <description>) is still referenced in line 22 ("creates a commit message using conventional commit format") but the actual format is no longer documented in the file.

Suggested change
**Types:**
## Conventional Commit Format
Commit messages created by this skill follow the Conventional Commits pattern:
`<type>[optional scope]: <description>`
**Types:** The `<type>` portion of the message can be one of:

Copilot uses AI. Check for mistakes.
3. **`fbcodereview`** extension must be enabled in Sapling config

## Important Notes

Copilot AI Feb 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the Prerequisites section may cause confusion for users who need to set up Phabricator integration. While this section is being removed from the skill docs, the prerequisites are still documented in other parts of the codebase (specs/source-control-type-selection.md, README.md) and are still actual requirements for using jf submit. Consider adding a brief note directing users to setup documentation, such as: "Note: Requires Phabricator configuration (.arcconfig). See repository setup documentation for details."

Suggested change
- Note: Requires Phabricator configuration (`.arcconfig`). See repository setup documentation for details.

Copilot uses AI. Check for mistakes.
3. **`fbcodereview`** extension must be enabled in Sapling config

## Important Notes

Copilot AI Feb 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the Prerequisites section may cause confusion for users who need to set up Phabricator integration. While this section is being removed from the skill docs, the prerequisites are still documented in other parts of the codebase (specs/source-control-type-selection.md, README.md) and are still actual requirements for using jf submit. Consider adding a brief note directing users to setup documentation, such as: "Note: Requires Phabricator configuration (.arcconfig). See repository setup documentation for details."

Suggested change
- Note: Requires Phabricator configuration (`.arcconfig`). See repository setup documentation for details.

Copilot uses AI. Check for mistakes.
3. **`fbcodereview`** extension must be enabled in Sapling config

## Important Notes

Copilot AI Feb 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the Prerequisites section may cause confusion for users who need to set up Phabricator integration. While this section is being removed from the skill docs, the prerequisites are still documented in other parts of the codebase (specs/source-control-type-selection.md, README.md) and are still actual requirements for using jf submit. Consider adding a brief note directing users to setup documentation, such as: "Note: Requires Phabricator configuration (.arcconfig). See repository setup documentation for details."

Suggested change
- Note: Requires Phabricator configuration (`.arcconfig`). See repository setup documentation for details.

Copilot uses AI. Check for mistakes.

1. If there are uncommitted changes, first run `/commit` to create a commit
2. Submit commits to Phabricator using `jf submit` (or `arc diff`)
2. Submit commits to Phabricator using `jf submit --draft`. Submit for review using DRAFT mode

Copilot AI Feb 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text "Submit for review using DRAFT mode" is redundant with the first part of the sentence. The sentence currently reads: "Submit commits to Phabricator using jf submit --draft. Submit for review using DRAFT mode". Consider simplifying to just: "Submit commits to Phabricator using jf submit --draft" or rewording to: "Submit commits to Phabricator in DRAFT mode using jf submit --draft"

Suggested change
2. Submit commits to Phabricator using `jf submit --draft`. Submit for review using DRAFT mode
2. Submit commits to Phabricator in DRAFT mode using `jf submit --draft`

Copilot uses AI. Check for mistakes.

1. If there are uncommitted changes, first run `/commit` to create a commit
2. Submit commits to Phabricator using `jf submit` (or `arc diff`)
2. Submit commits to Phabricator using `jf submit --draft`. Submit for review using DRAFT mode

Copilot AI Feb 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text "Submit for review using DRAFT mode" is redundant with the first part of the sentence. The sentence currently reads: "Submit commits to Phabricator using jf submit --draft. Submit for review using DRAFT mode". Consider simplifying to just: "Submit commits to Phabricator using jf submit --draft" or rewording to: "Submit commits to Phabricator in DRAFT mode using jf submit --draft"

Suggested change
2. Submit commits to Phabricator using `jf submit --draft`. Submit for review using DRAFT mode
2. Submit commits to Phabricator in DRAFT mode using `jf submit --draft`

Copilot uses AI. Check for mistakes.
Comment thread .claude/skills/sl-commit/SKILL.md Outdated
Comment on lines 46 to 48

**Types:**

Copilot AI Feb 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The section header "Types:" appears orphaned after removing the "Conventional Commits Format" section. This creates a documentation structure issue where types are listed without the format context that explains how to use them. Consider either:

  1. Adding a brief header like "## Conventional Commit Types" before this line
  2. Re-introducing a simplified format explanation before the types list
  3. Moving this types list under the "What This Skill Does" section with appropriate context

The conventional commit format pattern (<type>[optional scope]: <description>) is still referenced in line 22 ("creates a commit message using conventional commit format") but the actual format is no longer documented in the file.

Suggested change
**Types:**
## Conventional Commit Format
Conventional commit messages follow this pattern:
`<type>[optional scope]: <description>`
Example:
`feat(ui): add dark mode toggle`
### Conventional Commit Types

Copilot uses AI. Check for mistakes.

1. If there are uncommitted changes, first run `/commit` to create a commit
2. Submit commits to Phabricator using `jf submit` (or `arc diff`)
2. Submit commits to Phabricator using `jf submit --draft`. Submit for review using DRAFT mode

Copilot AI Feb 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text "Submit for review using DRAFT mode" is redundant with the first part of the sentence. The sentence currently reads: "Submit commits to Phabricator using jf submit --draft. Submit for review using DRAFT mode". Consider simplifying to just: "Submit commits to Phabricator using jf submit --draft" or rewording to: "Submit commits to Phabricator in DRAFT mode using jf submit --draft"

Suggested change
2. Submit commits to Phabricator using `jf submit --draft`. Submit for review using DRAFT mode
2. Submit commits to Phabricator in DRAFT mode using `jf submit --draft`

Copilot uses AI. Check for mistakes.
The conventional commit types are already documented in the full
Conventional Commits specification referenced by the skill. Removing the
duplicate list reduces maintenance burden and avoids drift.

Assistant-model: Claude Code
@claude

claude Bot commented Feb 23, 2026

Copy link
Copy Markdown

PR Review: Simplify sl-commit and sl-submit-diff skill docs

Overall Assessment

This is a well-structured documentation cleanup PR that removes redundant content and streamlines the Sapling skill documentation. The changes are applied consistently across all three agent configurations (.claude/, .github/, .opencode/), which is good practice.

✅ Positives

  1. Removes duplicate content: The Conventional Commits format section is already comprehensively documented in gh-commit/SKILL.md, so removing it from sl-commit reduces maintenance burden and keeps documentation DRY.

  2. Consistent changes: All three agent skill directories receive identical updates, ensuring consistent behavior across Claude Code, GitHub Copilot, and OpenCode.

  3. Clear PR description: The changes and rationale are well-documented in the PR body.

  4. Net reduction in documentation: -114 lines of duplication is a clean improvement.

⚠️ Suggestions

  1. Cross-reference for Conventional Commits: After removing the Conventional Commits format from sl-commit, consider adding a brief note like:

    - Follow Conventional Commits format (see `/commit` or `gh-commit` skill for details)

    This helps users of the Sapling workflow find the commit message format documentation.

  2. Draft mode as default: The change from jf submit to jf submit --draft as the default command is a workflow change, not just documentation cleanup. Consider:

    • Is this the intended default for all users?
    • Should there be a note about how to submit for immediate review (without --draft) when needed?

    For example, adding a row to the "Common Operations" table:

    | Submit ready for review | `jf submit`                       |
  3. Smartlog reference inconsistency: The PR removes sl smartlog documentation from sl-commit, but sl ssl (a smartlog alias) is still referenced in sl-submit-diff:

    - `sl ssl` - View commits with diff status

    Users unfamiliar with Sapling might not know that ssl is short for smartlog. Consider either keeping a brief smartlog mention or clarifying what ssl stands for.

  4. Prerequisites removal consideration: The Prerequisites section being removed contained useful setup information:

    • .arcconfig requirement
    • ~/.arcrc credentials
    • fbcodereview extension

    If this info exists elsewhere in the repository or is truly outdated for Meta's workflow, the removal is fine. Otherwise, new users might encounter confusing errors without guidance on setup.

🔒 Security & Performance

No security or performance concerns - this is purely documentation changes.

📝 Summary

Recommendation: Approve with minor suggestions. The core cleanup is valuable and the changes are well-organized. The suggestions above are optional improvements that could enhance clarity for users transitioning between Git and Sapling workflows.

@claude

claude Bot commented Feb 23, 2026

Copy link
Copy Markdown

PR Review: Simplify sl-commit and sl-submit-diff skill docs

Thanks for this documentation cleanup! The goal of reducing redundancy and focusing on the recommended workflow is sound. Here's my detailed review:

✅ What Works Well

  1. Consistent changes across all agent configs - Changes are applied uniformly to .claude/skills/, .github/skills/, and .opencode/skills/
  2. Removing duplicate Conventional Commits section - This was redundant since the specification is already referenced; reducing maintenance burden is valuable
  3. Simplifying sl diff --stat to sl diff - Cleaner and the --stat flag isn't essential for the workflow

⚠️ Concerns & Suggestions

1. Forcing --draft mode as the only option

The PR standardizes on jf submit --draft, but users may want to submit directly for review without draft mode. Consider:

  • Documenting both jf submit and jf submit --draft with a note that --draft is recommended
  • Or add a note explaining when to use non-draft mode

Current:

| Submit current commit   | `jf submit --draft`               |

Suggestion:

| Submit as draft         | `jf submit --draft` (recommended) |
| Submit for review       | `jf submit`                        |

2. Removing sl ssl may impact usability

sl ssl (smartlog with status) is useful for checking diff status in the terminal. Users will need a way to verify submission status. Consider keeping:

- `sl ssl` - View commits with diff status

3. Removing status query commands

The removed commands were useful for scripting and verification:

| View diff status        | `sl ssl`                          |
| Check sync status       | `sl log -T '{syncstatus}\n' -r .` |
| Get diff ID             | `sl log -T '{phabdiff}\n' -r .`   |

If these are considered advanced usage, perhaps move them to an "Advanced" section rather than removing entirely?

4. Removing Prerequisites section

While this may be outdated, new users setting up Phabricator integration still need to know about:

  • .arcconfig in repository root
  • ~/.arcrc credentials
  • fbcodereview extension

Consider either:

  • Keeping a simplified version with a link to official setup docs
  • Moving to a separate setup guide document

5. Removing sl absorb documentation

sl absorb is a powerful feature for stacked diffs workflows. Removing it from documentation reduces discoverability. Consider keeping at least a brief mention in "Key Sapling Differences".

📝 Minor Issues

  • Line 18 in sl-submit-diff/SKILL.md has slightly awkward phrasing: "Submit commits to Phabricator using jf submit --draft. Submit for review using DRAFT mode" - the second sentence is redundant

🔍 No Security/Performance Concerns

This is documentation-only, so no security or performance issues.

📋 Test Coverage

N/A - documentation changes don't require tests.


Summary: The cleanup is directionally correct, but I'd recommend reconsidering the complete removal of sl ssl, status query commands, and the option to submit without --draft. A lighter approach would preserve these as "advanced" or "optional" rather than removing them entirely.

@flora131
flora131 merged commit 0f75e23 into main Feb 23, 2026
2 checks passed
@claude claude Bot mentioned this pull request Feb 25, 2026
@lavaman131
lavaman131 deleted the flora131/feature/update-sl-commands branch March 19, 2026 15:42
lavaman131 pushed a commit that referenced this pull request Mar 26, 2026
…ands

chore(skills): simplify sl-commit and sl-submit-diff skill docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants