Skip to content

Add builtin_items updater to whats_left job #6238

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.

Sign up for GitHub

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

Jump to bottom
Merged
youknowone merged 1 commit into from
Nov 9, 2025
Merged

Add builtin_items updater to whats_left job #6238

youknowone merged 1 commit into from
Nov 9, 2025

Conversation

@fanninpm
Copy link
Contributor

@fanninpm fanninpm commented Nov 6, 2025 •
edited by coderabbitai bot
Loading

Corresponds to RustPython/rustpython.github.io#81.

Summary by CodeRabbit

  • Chores
    • Updated CI workflow configuration with improved command formatting and scheduling precision.
    • Enhanced data processing pipeline to generate structured data exports during build processes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025 •
edited
Loading

Walkthrough

A GitHub Actions workflow configuration file is updated to add proper quoting around a cron expression, remove trailing whitespace from a command, and introduce AWK-based CSV generation steps to transform builtin items data in two workflow paths.

Changes

Cohort / File(s) Summary
Workflow configuration updates
.github/workflows/cron-ci.yaml		 Added double quotes around cron expression for consistency; removed trailing whitespace after --fail-env-changed flag; introduced two AWK-based data transformations to generate builtin_items.csv from _data/whats_left.temp, outputting CSV with columns: builtin, name, is_inherited

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • AWK script logic: Verify the AWK parsing logic correctly identifies the "# builtin items" marker and extracts/transforms data into the expected CSV format
  • CSV output consistency: Confirm the generated CSV schema (builtin, name, is_inherited columns) matches downstream consumer expectations in both workflow paths

Poem

🐰 A workflow refined with quotes so bright,
Trailing spaces trimmed with all our might,
AWK scripts dance through data streams,
Building CSVs of automation dreams,
Cron ticks on, precise and true—
CI pipelines shining fresh and new! ✨

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding a builtin_items updater to the whats_left job, which aligns with the core modification of adding AWK-based CSV generation in the workflow.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5cad66c and 9181a78.

📒 Files selected for processing (1)
  • .github/workflows/cron-ci.yaml (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings 
Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Check the WASM package and demo
  • GitHub Check: Ensure compilation on various targets
  • GitHub Check: Check Rust code with rustfmt and clippy
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Run snippets and cpython tests (windows-latest)
  • GitHub Check: Run rust tests (windows-latest)
  • GitHub Check: Run rust tests (macos-latest)
  • GitHub Check: Run rust tests (ubuntu-latest)
🔇 Additional comments (3)
.github/workflows/cron-ci.yaml (3)

3-3: Proper quoting of cron expression.

Good practice to quote cron expressions in YAML to prevent potential parser misinterpretation. No functional change to the schedule.

37-37: Trailing whitespace cleanup.

Minor formatting improvement; no functional change to the command.

112-128: Well-structured AWK script for CSV transformation.

The AWK script correctly:

  • Parses the builtin items section marked by # builtin items
  • Extracts the builtin module name (first component of dotted name via split($1, a, "."))
  • Captures metadata text after the first space
  • Outputs CSV format with appropriate header

The heredoc syntax with single-quoted EOF is correct and prevents variable expansion. The script handles edge cases appropriately (missing dots, missing spaces, empty lines marking section boundaries).

One clarification: the AI summary mentions this is added "in two workflow paths" but I only see this AWK addition in the whatsleft job. Can you confirm whether another similar AWK-based transformation should be added elsewhere in the workflow, or if the summary was describing the overall intent?

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@fanninpm fanninpm marked this pull request as ready for review November 6, 2025 03:58
@fanninpm fanninpm requested a review from youknowone November 6, 2025 14:48
fanninpm
fanninpm commented Nov 7, 2025
View reviewed changes
Copy link
Contributor Author

@fanninpm fanninpm left a comment

Choose a reason for hiding this comment

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

These lines were changed due to Zed's auto-formatting. @youknowone should we keep the changes or put them back to the way they were?

.github/workflows/cron-ci.yaml
on:
schedule:
- cron: '0 0 * * 6'
- cron: "0 0 * * 6"
Copy link
Contributor Author

@fanninpm fanninpm Nov 7, 2025

Choose a reason for hiding this comment

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

Suggested change
- cron: "0 0 * * 6"
- cron: '0 0 * * 6'

.github/workflows/cron-ci.yaml
continue-on-error: true
- name: Run cargo-llvm-cov with Python test suite.
run: cargo llvm-cov --no-report run -- -m test -u all --slowest --fail-env-changed
run: cargo llvm-cov --no-report run -- -m test -u all --slowest --fail-env-changed
Copy link
Contributor Author

@fanninpm fanninpm Nov 7, 2025

Choose a reason for hiding this comment

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

Suggested change
run: cargo llvm-cov --no-report run -- -m test -u all --slowest --fail-env-changed
run: cargo llvm-cov --no-report run -- -m test -u all --slowest --fail-env-changed

youknowone
youknowone approved these changes Nov 9, 2025
View reviewed changes
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

@fanninpm following the auto-formatting will reduce future inconvenience. Nothing bad, especially about CI configs. because bisect to CI configs usually doesn't make sense

@youknowone youknowone merged commit ed9a61d into RustPython:main Nov 9, 2025
12 checks passed
@fanninpm fanninpm deleted the whats-left-builtin-items branch November 11, 2025 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fanninpm @youknowone