Skip to content

Add missing job-status options to usage #108

Merged
merged 1 commit into from Sep 1, 2021

Conversation

niclas
Copy link
Contributor

@niclas niclas commented Aug 30, 2021

There were some job-status options missing in the documentation/usage. I added these which should clarify and resolve #74 (mxqdump -j -g NNNN -s all).

@niclas niclas requested a review from donald August 30, 2021 14:52
@donald donald requested a review from pmenzel August 31, 2021 05:21
@pmenzel
Copy link
Contributor

pmenzel commented Aug 31, 2021

Thank you for the patch. Please add the description also to the commit message (How to Write a Git Commit Message).

Could you please also add a short option/switch for --status=all for job options? Maybe -A? That would fix

The documentation of

  -g, --group-id <group-id>      filter jobs in group with <group-id>

could be extended with (without -s only jobs with job status running). That would fix issue #74.

Copy link
Contributor

@pmenzel pmenzel left a comment

Choose a reason for hiding this comment

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

As written in the issue, -a (arg_all) should be documented in the usage.

@niclas
Copy link
Contributor Author

niclas commented Aug 31, 2021

As written in the issue, -a (arg_all) should be documented in the usage.

@pmenzel, now you made me confused. -a is currently documented here.

Regarding the shortcut: I can use -A but the long option has to be something different than --all because that already exists. Do you have any suggestions?

@pmenzel
Copy link
Contributor

pmenzel commented Aug 31, 2021

As written in the issue, -a (arg_all) should be documented in the usage.

@pmenzel, now you made me confused. -a is currently documented here.

It’s documented under groups-options and not jobs-options. Due to the indentation and blank lines in between, I think, it does not apply to jobs-options.

Regarding the shortcut: I can use -A but the long option has to be something different than --all because that already exists. Do you have any suggestions?

-a,/--all works already I guess, and probably can be considered an alias despite different code paths are taken.

@donald
Copy link
Contributor

donald commented Aug 31, 2021

I still don't get it and I are not sure how the code is supposed to work. @pmenzel: Can you give a complete example of a command which is working but is not shown in the usage?

Copy link
Contributor

@pmenzel pmenzel left a comment

Choose a reason for hiding this comment

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

I am going to send a separate merge/pull request to fix my issue.

@pmenzel pmenzel dismissed their stale review September 1, 2021 10:32

I am going to send a separate merge/pull request to fix my issue.

mxqdump.c Show resolved Hide resolved
@donald donald merged commit 597e320 into master Sep 1, 2021
@donald donald deleted the improve-mxqdump-job-status-usage branch October 28, 2022 14:21
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mxqdump: Dump all jobs from group by default without filtering
3 participants