Skip to content

add default message for pvget - #195

Open
rerpha wants to merge 1 commit into
epics-base:masterfrom
rerpha:patch-1
Open

add default message for pvget#195
rerpha wants to merge 1 commit into
epics-base:masterfrom
rerpha:patch-1

Conversation

@rerpha

@rerpha rerpha commented Apr 16, 2024

Copy link
Copy Markdown

Adds a default message for pvget. I don't know if line 363 (the default case) was a bug as i could never get it to run. @ralphlange I found this after I showed you my (accidentally?) working PVAccess.

Maybe there's a good reason that pvget doesn't have this line but the other tools do, please let me know!

@AppVeyorBot

Copy link
Copy Markdown

@AppVeyorBot

Copy link
Copy Markdown

@AppVeyorBot

Copy link
Copy Markdown

@mdavidsaver

Copy link
Copy Markdown
Member

I don't know if line 363 (the default case) was a bug as i could never get it to run.

That default case should be impossible so long as the switch is consistent with the string passed to getopt(). imo. any inconsistency should be an error. I usually combine default and '?' though.

Maybe there's a good reason that pvget doesn't have this line but the other tools do, please let me know!

I guess you are referring to caget?

It think it is a bit of a judgement call whether PVs is an error with pvget. With pvmonitor I think this should be.

Is it worth changing behavior at this point?

@rerpha

rerpha commented May 20, 2024

Copy link
Copy Markdown
Author

Maybe there's a good reason that pvget doesn't have this line but the other tools do, please let me know!

I guess you are referring to caget?

It think it is a bit of a judgement call whether PVs is an error with pvget. With pvmonitor I think this should be.

Is it worth changing behavior at this point?

by other tools yes caget, but also pvput, pvinfo, pvmonitor all seem to give the "No pv name(s) specified. ('pvXXXX -h' for help.)" message - just thinking for consistency among the pv* utils it might be nice.

Comment thread pvtoolsSrc/pvget.cpp
@AppVeyorBot

Copy link
Copy Markdown

Comment thread pvtoolsSrc/pvget.cpp Outdated
@rerpha
rerpha requested a review from anjohnson April 9, 2025 10:42
@AppVeyorBot

Copy link
Copy Markdown

Build pvAccessCPP 1.0.116 failed (commit 9a1e4a8a81 by @rerpha)

@anjohnson anjohnson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank-you, this looks fine now.

Just a note for any future Pull Requests you make that have more complex changes and more than one commit: Please don't use git pull after committing changes, since that adds a merge commit to your PR branch. Our preferred method of moving your changes up to the latest commit on the 7.0 branch is to do a git pull --rebase. In this case though we will squash your changes into a single commit when we merge it so you don't need to do anything more here.

@AppVeyorBot

Copy link
Copy Markdown

Build pvAccessCPP 1.0.117 failed (commit bd8810429a by @rerpha)

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results
Complexity 0
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes. Give us feedback

@AppVeyorBot

Copy link
Copy Markdown

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