Skip to content

Conversation

igorpecovnik
Copy link
Member

@igorpecovnik igorpecovnik commented Sep 29, 2025

Description

Inserting firstboot.conf from userpatches/ folder.

Documentation summary for feature / change

Config usage: https://docs.armbian.com/User-Guide_Autoconfig/

How Has This Been Tested?

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

@igorpecovnik igorpecovnik requested a review from a team as a code owner September 29, 2025 18:46
@igorpecovnik igorpecovnik added the Work in progress Unfinished / work in progress label Sep 29, 2025
@github-actions github-actions bot added size/small PR with less then 50 lines 11 Milestone: Fourth quarter release labels Sep 29, 2025
Copy link
Contributor

coderabbitai bot commented Sep 29, 2025

Walkthrough

  • In lib/functions/rootfs/distro-agnostic.sh, new logic checks for ${USERPATCHES_PATH}/firstboot.conf.
  • If the file exists, it prints an informational message and copies it to ${SDCARD}/root/.not_logged_in_yet.
  • This step is executed before enabling desktop autologin.
  • No public/exported declarations were added or modified.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "Introduce including user provided first-boot config at build time" accurately describes the main change in the changeset. The code modifications add logic to check for and copy a user-provided firstboot.conf file from the userpatches directory to the build output, which is precisely what the title conveys. The title is clear, concise, and provides enough specificity for a teammate to understand the primary purpose of this change when scanning the project history.
Description Check ✅ Passed The pull request description is directly related to the changeset. It describes inserting firstboot.conf from the userpatches/ folder, which matches the code changes that add logic to copy this configuration file during the build process. The description also provides a helpful documentation reference for the config usage and includes a standard checklist, making it sufficiently informative about the change being made.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sdk

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6a08b23 and 5a8c30d.

📒 Files selected for processing (1)
  • lib/functions/rootfs/distro-agnostic.sh (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/functions/rootfs/distro-agnostic.sh

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
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

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

@github-actions github-actions bot added Needs review Seeking for review Framework Framework components labels Sep 29, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
action.yml (1)

15-19: Consider splitting unrelated changes into separate PRs.

This PR's objective is to "Introduce including user provided first-boot config at build time," but these changes add release artifact cleanup functionality, which appears unrelated to firstboot configuration.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between badbc99 and 6a08b23.

📒 Files selected for processing (2)
  • action.yml (2 hunks)
  • lib/functions/rootfs/distro-agnostic.sh (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/functions/rootfs/distro-agnostic.sh (1)
.github/generate_CODEOWNERS.sh (1)
  • display_alert (6-6)
🔇 Additional comments (2)
action.yml (1)

215-215: LGTM!

The usage of inputs.armbian_release_clean for removeArtifacts is correct and properly defaults to "false".

lib/functions/rootfs/distro-agnostic.sh (1)

112-116: [scratchpad]

The destination path for firstboot.conf is correct.

The review comment incorrectly assumes that .not_logged_in_yet should be a separate marker file and that firstboot.conf should be copied to a different location.

Analysis of the armbian-firstlogin script shows that /root/.not_logged_in_yet serves a dual purpose:

  1. Its presence triggers the first-login logic (checked at line 613: if [[ -f /root/.not_logged_in_yet && -n $(tty) ]]; then)
  2. It contains bash variable definitions that are sourced (line 93: source "$fp_config") to configure the automated setup

The script validates and sources this file as a bash script (lines 87-93), loading PRESET_* variables. The preset-firstrun.sh extension demonstrates this pattern by writing configuration variables directly to .not_logged_in_yet (lines 4-51).

Therefore, overwriting the empty marker file created at line 110 with the user's firstboot.conf at line 115 is the intended behavior.

Likely an incorrect or invalid review comment.

@igorpecovnik igorpecovnik added the Needs Documentation New feature needs documentation entry label Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

11 Milestone: Fourth quarter release Framework Framework components Needs Documentation New feature needs documentation entry Needs review Seeking for review size/small PR with less then 50 lines Work in progress Unfinished / work in progress

Development

Successfully merging this pull request may close these issues.

1 participant