Skip to content

Conversation

@mmaddala
Copy link
Contributor

PR Template and Checklist

Please complete as much as possible to speed up the reviewing process.

Readiness and adding reviewers as appropriate is required.

All PRs should be reviewed by a technical writer/documentation team and a peer.
If effecting customers—which is a majority of content changes—a member of Customer Success must also review.

Readiness

  • Merge (pending reviews)
  • Merge after date or event
  • Draft

Overview

Why merge this PR? What does it solve?

Checklist

  • Run spelling and grammar check, preferably with linter.
  • Avoid changing any header associated with a link/reference.
  • Step through instructions (or ask someone to do so).
  • Review for wordiness
  • Match tone and style of page/section.
  • Run make linkcheck.
  • View HTML in a browser to check rendering.
  • Use semantic newlines.
  • follow best practices for commits.
    • Descriptive title written in the imperative.
    • Include brief overview of QA steps taken.
    • Mention any related issues numbers.
    • End message with sign off/DCO line (-s, --signoff).
    • Sign commit with your gpg key (-S, --gpg-sign).
    • Squash commits if needed.
  • Request PR review by a technical writer and at least one peer.

Comments

Any thing else that a maintainer/reviewer should know.
This could include potential issues, rational for approach, etc.

@mmaddala mmaddala requested review from a team, caiotpereira and vanmaegima August 19, 2025 14:56
-device virtio-net-device,netdev=net0,mac=52:54:00:12:35:02 \
-device virtio-serial-device \
-drive id=disk0,file=lmp-factory-image-qemuarm64-secureboot.wic,if=none,format=raw \
-drive id=disk0,file=lmp-base-console-image-qemuarm64-secureboot.wic,if=none,format=raw \
Copy link
Member

Choose a reason for hiding this comment

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

this can be dropped
lmp-factory-image is the image built from a factory
lmp-base-console-image is the initial build on a new factory
this is covered by a note in this page

@vanmaegima
Copy link
Member

@mmaddala as good practices, please also update the commit title and add a little bit of explanation on why this change is needed
please check other commits in the git log for references on how this should be updated

@mmaddala
Copy link
Contributor Author

Updated the changes as suggested. Please look into them and let me know.

@vanmaegima
Copy link
Member

@mmaddala That looks neat. I forgot about another best practice: keep your commit line up to 80 characters.
For example 381ae4a and 08698e4, see how the commit lines are broken at around 80 mark.
Let's rework that and we should be able to merge it soon. Please note that @kprosise does the merges in this repo.

@mmaddala
Copy link
Contributor Author

@vanmaegima Thanks for letting me know. I have changed the commit line up to 80 characters.

@vanmaegima
Copy link
Member

@mmaddala thanks Manjusha! I do notice some typos in the new commit title, if you can fix that I will approve it.

@mmaddala
Copy link
Contributor Author

@vanmaegima fixed the typos in the new commit line.

@vanmaegima
Copy link
Member

@mmaddala the typo is still there: "for CLI command..."

Running the QEMU CLI command from the document was opening a QEMU
terminal, which should not happen. This was caused by a missing '\'
that broke the command. The missing character has been added to fix
the issue.
@mmaddala
Copy link
Contributor Author

@vanmaegima fixed the changes.

Copy link
Member

@vanmaegima vanmaegima left a comment

Choose a reason for hiding this comment

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

lgtm

@vanmaegima
Copy link
Member

@kprosise ready to merge

@caiotpereira
Copy link
Contributor

LGTM, Thanks @mmaddala and @vanmaegima

Copy link
Contributor

@kprosise kprosise left a comment

Choose a reason for hiding this comment

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

LGTM

@kprosise kprosise merged commit b105f41 into foundriesio:main Aug 21, 2025
1 of 3 checks passed
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.

4 participants