Skip to content

feat: Add YScope PR template and PR title check workflow. - #6

Closed
anlowee wants to merge 3 commits into
mainfrom
xwei/add-pr-template-and-title-check
Closed

feat: Add YScope PR template and PR title check workflow.#6
anlowee wants to merge 3 commits into
mainfrom
xwei/add-pr-template-and-title-check

Conversation

@anlowee

@anlowee anlowee commented Jun 5, 2025

Copy link
Copy Markdown

As the title says.

Summary by CodeRabbit

Test PR: https://github.com/anlowee/velox/pull/2/checks

Summary by CodeRabbit

  • Chores
    • Added a pull request template to guide contributors in creating well-structured PRs, including sections for descriptions, checklists, and validation details.
    • Introduced an automated workflow to enforce conventional commit standards on pull request titles, improving consistency and quality of contributions.
    • Removed the previous PR title validation job, streamlining the workflow for title checks.

@coderabbitai

coderabbitai Bot commented Jun 5, 2025

Copy link
Copy Markdown
## Walkthrough

A pull request template and a GitHub Actions workflow have been added. The template standardizes pull request submissions with sections and guidelines, while the workflow enforces that pull request titles conform to the Conventional Commits specification using an automated check on relevant pull request events. Additionally, a previous PR title validation job was removed from another workflow.

## Changes

| File(s)                                             | Change Summary                                                                                                 |
|-----------------------------------------------------|---------------------------------------------------------------------------------------------------------------|
| .github/PULL_REQUEST_TEMPLATE.md                    | Added a pull request template with sections for title guidance, description, checklist, breaking changes, documentation, and test validation. Disabled markdownlint rule MD012. |
| .github/workflows/pr-title-checks.yaml              | Added a workflow to validate pull request titles against the Conventional Commits specification using an external action.                  |
| .github/workflows/preliminary_checks.yml            | Removed the `title-check` job that previously validated PR titles with a custom Python script enforcing conventional commit format.           |

## Sequence Diagram(s)

```mermaid
sequenceDiagram
    participant Contributor
    participant GitHub
    participant PR Title Check Workflow

    Contributor->>GitHub: Opens/edits/reopens PR targeting main
    GitHub->>PR Title Check Workflow: Trigger workflow (pull_request_target)
    PR Title Check Workflow->>amannn/action-semantic-pull-request: Validate PR title
    amannn/action-semantic-pull-request-->>PR Title Check Workflow: Validation result
    PR Title Check Workflow-->>GitHub: Report status on PR

<!-- walkthrough_end -->


---

<details>
<summary>📜 Recent review details</summary>

**Configuration used: CodeRabbit UI**
**Review profile: ASSERTIVE**
**Plan: Pro**


<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 09817d4127d9e43212f67a964d9149e1416aecfe and f7ad71e0222d650f9446120a0e581fb5dbd13892.

</details>

<details>
<summary>📒 Files selected for processing (2)</summary>

* `.github/workflows/pr-title-checks.yaml` (1 hunks)
* `.github/workflows/preliminary_checks.yml` (0 hunks)

</details>

<details>
<summary>💤 Files with no reviewable changes (1)</summary>

* .github/workflows/preliminary_checks.yml

</details>

<details>
<summary>🔇 Additional comments (4)</summary><blockquote>

<details>
<summary>.github/workflows/pr-title-checks.yaml (4)</summary>

`1-2`: **Descriptive workflow name**  
The `name: "pr-title-checks"` is clear and aligns with the file’s purpose.

---

`13-13`: **Lock down default permissions**  
Explicitly setting `permissions: {}` at the top level follows best practices by removing all defaults. Job-level overrides then grant only what’s needed.

---

`21-26`: **Verify required job permissions**  
The `conventional-commits` job grants `pull-requests: read`. Confirm whether the action needs additional scopes (e.g. `pull-requests: write` or `issues: write`) to post comments or status checks on failure.

---

`3-12`: <details>
<summary>❓ Verification inconclusive</summary>

**Review `pull_request_target` usage for untrusted code**  
Using `pull_request_target` is valid for accessing secrets on forked PRs, but double-check that neither this workflow nor `amannn/action-semantic-pull-request` ever checks out or executes untrusted PR code. If secrets aren’t needed, consider switching to `pull_request` to minimise risk.

---

🏁 Script executed:

```shell
#!/bin/bash
set -e

echo "=== Display workflow ==="
cat .github/workflows/pr-title-checks.yaml

echo
echo "=== Looking for checkout usage ==="
grep -R "uses:.*checkout" -n .github/workflows/pr-title-checks.yaml || echo "No checkout found"

echo
echo "=== Looking for semantic PR action ==="
grep -R "uses:.*action-semantic-pull-request" -n .github/workflows/pr-title-checks.yaml || echo "No semantic PR action found"

echo
echo "=== Checking permissions section ==="
grep -R "permissions:" -n .github/workflows/pr-title-checks.yaml || echo "No permissions section"

echo
echo "=== Checking for explicit secrets usage ==="
grep -R "secrets\." -n .github/workflows/pr-title-checks.yaml || echo "No explicit secrets usage"

Length of output: 1938


Review pull_request_target usage for untrusted code
We’ve confirmed in .github/workflows/pr-title-checks.yaml that:

  • There is no actions/checkout step.
  • Global permissions are {} and the job only has pull-requests: read.
  • The only external action is amannn/action-semantic-pull-request@v5 using GITHUB_TOKEN.

However, please verify that amannn/action-semantic-pull-request does not check out or execute any untrusted PR code. If you don’t actually need elevated secrets access, consider switching the trigger to pull_request to reduce risk.


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🔭 Outside diff range comments (1)
.github/workflows/pr-title-checks.yaml (1)

27-31: 🧹 Nitpick (assertive)

Add descriptive step name
Including a name: for the semantic PR title check step (e.g. name: "Validate PR title") improves readability in the workflow logs.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 430fda0 and 09817d4.

📒 Files selected for processing (2)
  • .github/PULL_REQUEST_TEMPLATE.md (1 hunks)
  • .github/workflows/pr-title-checks.yaml (1 hunks)
🔇 Additional comments (2)
.github/workflows/pr-title-checks.yaml (1)

4-9: Ensure safe usage of pull_request_target
You’ve correctly scoped jobs to avoid unintended code execution when using pull_request_target. Ensure no checkout or execution of untrusted PR code in any subsequent steps.

.github/PULL_REQUEST_TEMPLATE.md (1)

3-14: Great template for title guidance
The initial comments clearly outline the imperative form and Conventional Commits rules.

Comment on lines +15 to +20
concurrency:
group: "${{github.workflow}}-${{github.ref}}"

# Cancel in-progress jobs for efficiency
cancel-in-progress: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Review concurrency grouping
Grouping by github.workflow and github.ref cancels outdated runs per branch. Consider scoping by PR number (github.event.pull_request.number) if you anticipate multiple workflows on the same branch.

🤖 Prompt for AI Agents
In .github/workflows/pr-title-checks.yaml around lines 15 to 20, the concurrency
group is currently set to use github.workflow and github.ref, which cancels
outdated runs per branch. To better scope concurrency when multiple workflows
run on the same branch, update the concurrency group to include
github.event.pull_request.number so that runs are grouped and canceled per PR
instead of per branch.

Comment on lines +22 to +30
# Checklist

<!-- Ensure each item below is satisfied and indicate so by inserting an `x` within each `[ ]`. -->

* [ ] The PR satisfies the [contribution guidelines][yscope-contrib-guidelines].
* [ ] This is a breaking change and that has been indicated in the PR title, OR this isn't a
breaking change.
* [ ] Necessary docs have been updated, OR no docs need to be updated.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Enhance the checklist
Consider adding a bullet for “CI passes” or “Tests added” to ensure contributors validate functionality in addition to documentation.

🤖 Prompt for AI Agents
In .github/PULL_REQUEST_TEMPLATE.md around lines 22 to 30, the PR checklist
lacks an item to confirm that continuous integration (CI) checks pass or that
tests have been added. Add a new bullet point to the checklist for contributors
to mark that CI passes successfully or that relevant tests have been included,
ensuring validation of functionality alongside documentation updates.

Comment on lines +1 to +2
<!-- markdownlint-disable MD012 -->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Disable MD041 for markdownlint
Since the file begins with an HTML comment before the H1 heading, you may want to disable MD041 (first-line-heading) to prevent lint errors.

🤖 Prompt for AI Agents
In .github/PULL_REQUEST_TEMPLATE.md at lines 1 to 2, add a markdownlint disable
directive for MD041 to prevent lint errors caused by the initial HTML comment
before the first heading. Insert a comment like <!-- markdownlint-disable MD041
--> at the top of the file to disable the first-line-heading rule.

@kirkrodrigues
kirkrodrigues self-requested a review June 5, 2025 19:30
@anlowee anlowee closed this Jun 5, 2025
wraymo pushed a commit that referenced this pull request Jun 26, 2025
…ger-overflow (facebookincubator#13831)

Summary:
Pull Request resolved: facebookincubator#13831

This avoids the following errors:

```
fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_abs.h:56:41: runtime error: negation of -9223372036854775808 cannot be represented in type 'long'; cast to an unsigned type to negate this value to itself
    #0 0x000000346ce5 in std::abs(long) fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_abs.h:56
    #1 0x000000345879 in std::shared_ptr<facebook::velox::BiasVector<facebook::velox::test::EvalTypeHelper<long>::Type>> facebook::velox::test::VectorMaker::biasVector<long>(std::vector<std::optional<long>, std::allocator<std::optional<long>>> const&) fbcode/velox/vector/tests/utils/VectorMaker-inl.h:58
    #2 0x000000344d34 in facebook::velox::test::BiasVectorErrorTest::errorTest(std::vector<std::optional<long>, std::allocator<std::optional<long>>>) fbcode/velox/vector/tests/BiasVectorTest.cpp:39
    #3 0x00000033ec99 in facebook::velox::test::BiasVectorErrorTest_checkRangeTooLargeError_Test::TestBody() fbcode/velox/vector/tests/BiasVectorTest.cpp:44
    #4 0x7fe0a2342c46 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) fbsource/src/gtest.cc:2727
    #5 0x7fe0a234275d in testing::Test::Run() fbsource/src/gtest.cc:2744
    #6 0x7fe0a2345fb3 in testing::TestInfo::Run() fbsource/src/gtest.cc:2890
    #7 0x7fe0a234c8eb in testing::TestSuite::Run() fbsource/src/gtest.cc:3068
    #8 0x7fe0a237b52b in testing::internal::UnitTestImpl::RunAllTests() fbsource/src/gtest.cc:6059
    #9 0x7fe0a237a0a2 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) fbsource/src/gtest.cc:2727
    #10 0x7fe0a23797f5 in testing::UnitTest::Run() fbsource/src/gtest.cc:5599
    #11 0x7fe0a2239800 in RUN_ALL_TESTS() fbsource/gtest/gtest.h:2334
    #12 0x7fe0a223952c in main fbcode/common/gtest/LightMain.cpp:20
    #13 0x7fe09ec2c656 in __libc_start_call_main /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #14 0x7fe09ec2c717 in __libc_start_main@GLIBC_2.2.5 /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../csu/libc-start.c:409:3
    #15 0x00000033d8b0 in _start /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../sysdeps/x86_64/start.S:116

UndefinedBehaviorSanitizer: signed-integer-overflow fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_abs.h:56:41
```
Avoid overflow by using the expression (static_cast<uint64_t>(1) + ~static_cast<uint64_t>(min)) to calculate the absolute value of min without using std::abs

Reviewed By: dmm-fb, peterenescu

Differential Revision: D76901449

fbshipit-source-id: 7eb3bd0f83e42f44cdf34ea1759f3aa9e1042dae
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.

1 participant