Skip to content

Rename option --threads to --processors #107

Merged
merged 2 commits into from Aug 30, 2021

Conversation

niclas
Copy link
Contributor

@niclas niclas commented Aug 26, 2021

This fixes #88.

--group-id was former used for --group-name. That prohibited the use of --group-id for -g.
@niclas niclas requested a review from donald August 26, 2021 13:24
@pmenzel
Copy link
Contributor

pmenzel commented Aug 26, 2021

I do not understand “--group-id was former used for --group-name. That prohibited the use of --group-id for -g.”. Please give an exact command, which does not work.

mxqsub.c Show resolved Hide resolved
mxqsub.c Outdated
@@ -904,9 +897,11 @@ int main(int argc, char *argv[])
}
break;

case 1:
mx_log_warning("option '--threads' is deprecated. please use '--processors' or '-j' in future calls.");
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Indentation incorrect?
  2. Please use …
  3. Maybe: option '--threads' was renamed to '--processors' and is deprecated. Please update your scripts.

Copy link
Contributor Author

@niclas niclas Aug 26, 2021

Choose a reason for hiding this comment

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

  1. I'll fix that.
  2. & 3. could be a knew pull request since I'm using the current style of the code. Therefore, I dislike to change that and leave the rest of the code.

@niclas
Copy link
Contributor Author

niclas commented Aug 26, 2021

I do not understand “--group-id was former used for --group-name. That prohibited the use of --group-id for -g.”. Please give an exact command, which does not work.

--group-id was used in former versions as option which is now --group-name but also the current -g has the long option --group-id. Thus, if you use --group-id, you'll maybe get the --group-name behavior. In any case the behavior depends on the order in which the arguments are listed in the source code. That's unacceptable.

@niclas niclas force-pushed the rename-option-threads-to-processors branch from 2bb1b78 to 048d25b Compare August 26, 2021 15:00
Copy link
Contributor

@donald donald left a comment

Choose a reason for hiding this comment

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

Good.

Side note: Maybe we should s/threads/processors/ in web/pages/mxq/mxq.in and https://twiki.molgen.mpg.de/foswiki/Edv/MarIuX64ClusterHowTo as well to avoid confusing users.

@wwwutz
Copy link
Contributor

wwwutz commented Aug 26, 2021

So may I ask out of the loop why I have to rewrite my submit-scripts ? Why even bother ? If it silently eats "--threads"that'll be ok with me. Why warn ? I you call something deprecated you should be preparing for a reuse of the option.

I'd prefer not to rewrite all my scripts just because upstream does things like this.

so: make it run silently, no warning, no docs.

@donald
Copy link
Contributor

donald commented Aug 26, 2021

so: make it run silently, no warning, no docs.

I suggested to emit a deprecated warning. But on a second thought I agree that we should accept --threads without warning.

@niclas niclas force-pushed the rename-option-threads-to-processors branch from 048d25b to 95f4a8e Compare August 26, 2021 21:04
@donald
Copy link
Contributor

donald commented Aug 27, 2021

But now --threads is gone? We wanted it to be silently accepted now. Or is this still WIP?

@niclas
Copy link
Contributor Author

niclas commented Aug 27, 2021

Should have done that at night. I just wasn't concentrated.

@niclas niclas force-pushed the rename-option-threads-to-processors branch from 95f4a8e to 32fd00d Compare August 27, 2021 09:05
@donald donald merged commit 66aeeb6 into master Aug 30, 2021
@donald donald deleted the rename-option-threads-to-processors 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.

RFC: Rename --threads to --cores ?
4 participants