Skip to content

Clarify docs and error strings for scsimon#1175

Merged
rdmark merged 2 commits intodevelopfrom
rdmark-issue-1158
Jun 24, 2023
Merged

Clarify docs and error strings for scsimon#1175
rdmark merged 2 commits intodevelopfrom
rdmark-issue-1158

Conversation

@rdmark
Copy link
Copy Markdown
Member

@rdmark rdmark commented May 21, 2023

  • Pull in some docs from wiki to man page
  • Clarify in error message that other processes can't use the gpio bus

@rdmark rdmark mentioned this pull request May 21, 2023
Closed
@rdmark rdmark requested a review from uweseimet June 19, 2023 20:47
uweseimet
uweseimet suggested changes Jun 19, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@uweseimet uweseimet left a comment

Choose a reason for hiding this comment

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

I don't think the error message should be changed. The new message is less precise and thus less helpful for the user. It does not give any hint which process may be the offending one, and the ordinary user would also not know what a GPIO bus is.

@rdmark rdmark force-pushed the rdmark-issue-1158 branch from dd5fa66 to 2f8ae68 Compare June 19, 2023 21:21
@rdmark rdmark requested a review from uweseimet June 19, 2023 21:21
@rdmark
Copy link
Copy Markdown
Member Author

rdmark commented Jun 19, 2023

@uweseimet I pushed a new revision now with a tweaked error message. What do you think?
The reason I want to change the error message, is that the current phrasing has proven ambiguous to at least one user.
The "Is PiSCSI or RaSCSI already running?" message isn't actionable, since it doesn't explicitly state which one is the correct state: running or not running.

akuker
akuker approved these changes Jun 20, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@akuker akuker left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you!

@uweseimet
Copy link
Copy Markdown
Contributor

uweseimet commented Jun 20, 2023

@rdmark I'm fine with the text now, but if just one user wants a change this should not automatically mean that something is going to be changed. All other users may have been fine with how it was before. What happens if after such a change another user (or two other users) want this change to be reverted? Do we revert it then? :) You cannot please everybody.

@rdmark rdmark merged commit ee658c3 into develop Jun 24, 2023
@rdmark rdmark deleted the rdmark-issue-1158 branch June 24, 2023 20:20
rdmark added a commit that referenced this pull request Sep 24, 2023
@rdmark
Clarify docs and error strings for scsimon (#1175)
c0bcb88 
* Better usage hints in scsimon man page

* In error message, clarify that piscsi needs to be shut down
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akuker akuker akuker approved these changes

@erichelgeson erichelgeson Awaiting requested review from erichelgeson

+1 more reviewer

@uweseimet uweseimet uweseimet approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rdmark @uweseimet @akuker