luci-app-attendedsysupgrade: Rewrite README.md for spelling, grammar, and clarity#8523
luci-app-attendedsysupgrade: Rewrite README.md for spelling, grammar, and clarity#8523Bruxism wants to merge 1 commit intoopenwrt:masterfrom
Conversation
| This app allows firmware upgrades of routers while keeping user installed packages. | ||
| To do so the app sends a request to an *Atttended SysUpgrade server* which will | ||
| respond with a custom image, containing all previously installed packages. | ||
| This app adds OpenWrt firmware upgrade and update as well as package update |
There was a problem hiding this comment.
This app adds OpenWrt firmware upgrade and update as well as package update functionality via its web interface.
I do find this sentence a little hard to read. I would suggest maybe rewording it to something like:
This app enables OpenWrt firmware upgrades and updates, as well as package updates, through its web interface (LuCI).
|
@owlsy, please let me know what you think of it. I was interested in making it succinct yet comprehensive. |
This comment has been minimized.
This comment has been minimized.
|
The commits need to be squashed into one commit. |
I'll learn how to and work on it now. Thanks. |
c4b80b9 to
40cc8f0
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Sure, please see my thoughts below.
If we want to match the typical way the OpenWrt Wiki seems to do it, I would replace:
with: System → Attended Sysupgrade
I would also calm the formatting down here a little bit, perhaps just a little with not including the italics, so it goes from: Attended SysUpgrade server (ASU) to: Attended SysUpgrade (ASU) server
I would also change the order of priority for what happens after the build request, so it would say something along the lines of: "Following a successful build request, the user is prompted to install
I would also keep the duration separate to the outage notice from being a sentence together, as well as include a note that this plugin has now been automatically included in OpenWrt as of 25.12.0: "The process for building a custom firmware image can take anywhere As of OpenWrt 25.12.0 and above,
Let's also clarify this wording so we know it's about the Attendedsysupgrade Server: "If you would like more information about the backend server (and how to |
|
Just to help you out as well, there's some requirements that need to be resolved for the pull request to be approved:
These two above are letting you know that you cannot use a noreply email for your commit. If you are not comfortable using a personal email address, you could create an alternative email address for situations like this. Then, change your email address for these checks to pass. Commit subject
In this case, your commit subject needs Based on your pull request title, your commit subject (and pull request title to rename) can be:
Commit message
Add a commit message. You can break the message up into multiple lines, and I believe less than 75 characters per line is the requirement. Based on the pull request comment, your commit message can be: |
This comment has been minimized.
This comment has been minimized.
|
Approved. The changes all look good to me. |
b07a45a to
b336f2b
Compare
luci-app-attendedsysupgrade
It appears at Build requests include information about the request system's By default, this plugin only prompts to send a build request when firmware Customizable build requests are directly available via the Following a successful build request, a prompt appears with information about ASU servers typically take between 30 seconds and 5 minutes Note Following the announcement of a major OpenWrt upgrade, availability of Further readingIf you would like more information about the backend server (and how to Support is available at the official
|
|
I was inspired by the triple Let me know if I went in a pleasant direction or if I should pare this down grossly to something that basically looks nearly like the top two paragraphs alone. I'm open to leaving it, to pare it down grossly, or to rewrite it with more markdown styling--refactoring it in a way. I threw in that note alert as a little flare in the presumption that would help curb any concerns during major upgrade announcements. It may be gaudy, but it's gaudy with a purpose. I say that half jokingly as I imagine it comes up too often in the support threads. Still, I included a link to the official support forum as it was mentioned in the OpenWrt wiki which I hope will be found suitable to be included in the README.md. Hopefully, the note filters out much of those concerns from ending up in the forum. @owlsy, as requested, I put in a note about the default inclusion of A part of me wanted to use a tip alert in the italicized line about By the way, @owlsy, with |
|
It looks great — I don't have any further suggestions, aside from removing some extra line breaks and trying to keep everything to the same width. See how I would do the line breaks.luci-app-attendedsysupgrade
It appears at Build requests include information about the request system's CPU By default, this plugin only prompts to send a build request when firmware Customizable build requests are directly available via the Following a successful build request, a prompt appears with information ASU servers typically take between 30 seconds and 5 minutes to process
Further readingIf you would like more information about the backend server (and how to host Support is available at the official LuCI Attended Sysupgrade support thread.
This would be better directed towards @efahl (who, by the way already provided a positive response to your changes) as they would have a greater understanding and authority for how luci-app-attendedsysupgrade functions, or know who else to ping and bring in for this type of discussion (not me though, I'm just a user like you). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
b3ada0e to
0232f73
Compare
|
Sorry for the messy pull request thread. I'm learning as I go here. @owlsy, thanks for the help! The breaks would be good, but that would be in the context of this thread. It doesn't look the same as when viewing it in the repo. I was mistaken that you were the maintainer, haha. I think I tagged you because of the rule to bring a reviewer and I thought I saw you as the last commit on this app. You've gone above and beyond for me helping me here with this! Thank you! @efahl please let me know your thoughts--particularly with the comments I had mistaking directed towards @owlsy in this comment. |
Ah, brilliant point. I didn't think of that!
You're very welcome, my thanks to you as well for letting me and taking it on board. I only just did a similar thing as well last week with my first ever pull request, so it felt right to help you out as well. :) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Your recent changes all look fine. We can commit this. |
| This app allows firmware upgrades of routers while keeping user installed packages. | ||
| To do so the app sends a request to an *Atttended SysUpgrade server* which will | ||
| respond with a custom image, containing all previously installed packages. | ||
| `luci-app-attendedsysupgrade` is a plugin for the OpenWrt LuCI web interface |
There was a problem hiding this comment.
"is an app" is preferred
There was a problem hiding this comment.
@systemcrash I had wondered about that. I figured that since these apps can't work without LuCI that technically they're plugins, so I went with 'plugin' instead. I did notice that all of these apps go under the directory 'Applications', yet I wasn't sure.
Change made as preferred.
README.md rewritten for improvements to spelling, grammar, and clarity. Signed-off-by: Bruxism <Bruxiandee@gmail.com>
0232f73 to
2d07d6a
Compare
Failed checksIssues marked with an ❌ are failing checks. Commit 2d07d6a
For more details, see the full job log. Something broken? Consider providing feedback. |
This PR is not from my main or master branch 💩, but a separate branch ✅
Each commit has a valid ✒️
Signed-off-by: <my@email.address>row (viagit commit --signoff) (not sure if the github provided one counts.)Each commit and PR title has a valid 📝
<package name>: titlefirst line subject for packagesIncremented 🆙 any
PKG_VERSIONin the Makefile (N/A)Tested on: (architecture, openwrt version, browser) (N/A)
( Preferred ) Mention: @ the original code author for feedback
@jow-
( Preferred ) Screenshot or mp4 of changes:
( Optional ) Closes: e.g. openwrt/luci#issue-number
( Optional ) Depends on: e.g. openwrt/packages#pr-number in sister repo
Description: (describe the changes proposed in this PR)
Changes were made for improvements to spelling, grammar, and clarity.