ci: publish Fedora RPM releases#1321
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bbe31044a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| test -f /opt/bin/lemonade-router | ||
|
|
||
| lemonade-server --version | ||
| lemonade-router --version |
There was a problem hiding this comment.
This smoke test invokes lemonade-router --version, but the Linux packaging rules install the router under /opt/bin and do not create a /usr/bin/lemonade-router symlink, so the command is typically not on Fedora's default PATH. Since release now depends on test-rpm-package, this can fail tag releases even when the RPM built correctly; invoke /opt/bin/lemonade-router (or export PATH) here.
Useful? React with 👍 / 👎.
| set -e | ||
|
|
||
| if command -v systemctl > /dev/null 2>&1 && [ -d /run/systemd/system ]; then | ||
| systemctl stop lemonade-server.service > /dev/null 2>&1 || true |
There was a problem hiding this comment.
The new %preun hook always executes systemctl stop lemonade-server.service, but %post only does daemon-reload, so upgrading from one RPM version to the next leaves the service stopped until an operator manually starts it. This creates avoidable downtime on systemd hosts; make the stop conditional to erase-only or add a restart/try-restart path in post-install for upgrades.
Useful? React with 👍 / 👎.
|
Status update for the Fedora RPM work on Review feedback addressed
Local package validation
Host-side Fedora validation
Upgrade-proof check
This gives us both packaging validation and real host-side evidence that the Fedora RPM installs, runs, preserves the FLM override, and upgrades without bouncing the service once the |
|
We should have the Fedora installation setup instructions on the website doc updated for this RPM! |
|
@jeremyfowers my feeling is we should include this as part of 10.0.0 if CI passes. |
|
@claude review |
| @@ -351,6 +351,64 @@ jobs: | |||
| path: build/lemonade-server_${{ env.LEMONADE_VERSION }}_amd64.deb | |||
There was a problem hiding this comment.
@OmerFarukOruc please replace the existing Fedora instructions in the Install Options website with a link to the RPM in the release, and provide a screenshot of the rendered page.
Currently looks like this:
Should look like this:
If the plan is to use the AppImage as an optional frontend, please include this text too:
| retention-days: 7 | ||
|
|
||
| build-lemonade-rpm: | ||
| name: Build Lemonade .rpm Package |
There was a problem hiding this comment.
Also please add a line here to add Fedora and the .rpm artifact to the tabe:
And remove the Fedora mention from the "Other platforms: ..." note.
Currently looks like this when rendered:
jeremyfowers
left a comment
jeremyfowers
left a comment
There was a problem hiding this comment.
@OmerFarukOruc I'm really excited for this!
Please address my two ui/ux comments, which should get us through most of the review checklist.
Also if claude comes back with any "critical" issues please address those. Other than that, we can merge!
|
@claude review |
|
Claude Code is working… I'll analyze this and get back to you. |
Our claude bot isnt working on forks at the moment. I ran it offline and its very happy with this PR :) So all we need is the documentation and then we're good to merge! |
|
v10 is happening now, so I'm going to merge this. We should add the documentation piece ASAP though. |
sorry i couldnt find time to check on this. if you guys want i can create a new pr for remaining reviews. 😞 😭 |
That next PR would be great, thank you @OmerFarukOruc! And don't sweat it, we've been doing final testing for the v10 release for days, so it was bound to surprise somebody. Just happened to be you on this PR. The good news is that our documentation and website can update independently of the release, so you have plenty of time. I just had to merge this PR ASAP to get the .rpm into the release. |
4 participants
Summary
Testing
bash -n src/cpp/postinst-rpm src/cpp/prerm-rpm src/cpp/postrm-rpmcpack -G RPM -Vrpm -qiprpm -qlpFollow-up to #1315 and docs PR #1320.
Review Checklist
Clarity
Completeness
lemonade-server --helpUI/UX review
Are the UI/UX changes (if any)...
Note: UI/UX includes installers, app, tray, and CLI.
Code quality