Skip to content

Conversation

jdvorak001
Copy link
Contributor

Describe your changes

Fix #2901

  • the datetime is fetched just once
  • it is then passed to the functions that start the output files, always as arg1

A question: Shouldn’t we put ’T’ between the date and the time instead of the dash? That way we’d be consistent with the ISO 8601 standard.

What is your pull request about?

  • Bug fix
  • Improvement
  • New feature (adds functionality)
  • Breaking change (bug fix, feature or improvement that would cause existing functionality to not work as expected)
  • Typo fix
  • Documentation update
  • Update of other files

If it's a code change please check the boxes which are applicable

  • For the main program: My edits contain no tabs, indentation is five spaces and any line endings do not contain any blank chars
  • I've read CONTRIBUTING.md and Coding_Convention.md
  • I have tested this fix or improvement against >=2 hosts and I couldn't spot a problem
  • I have tested this new feature against >=2 hosts which show this feature and >=2 host which does not (in order to avoid side effects) . I couldn't spot a problem
  • For the new feature I have made corresponding changes to the documentation and / or to help()
  • If it's a bigger change: I added myself to CREDITS.md (alphabetical order) and the change to CHANGELOG.md

- the datetime is fetched just once
- it is then passed to the functions that start the output files, always as arg1
@drwetter
Copy link
Collaborator

Hi @jdvorak001 ,

thanks for taking action!

A question: Shouldn’t we put ’T’ between the date and the time instead of the dash? That way we’d be consistent with the ISO 8601 standard.

How it should be like, is a matter of YMMV ;-) At the moment in this PR I wouldn't vote for that. The format could be configurable though some time in the future.

Remarks:

  • datetime_filename_part is too long and makes the code more difficult to read.
  • Don't capitalize the variables, unless they are global VARS (which need to declared in the very beginning).

@jdvorak001
Copy link
Contributor Author

Thanks for the comments on the style, I’ve adapted. However, I find datetime a bit too vague, so I’m suggesting datetime_started.

@drwetter
Copy link
Collaborator

drwetter commented Sep 26, 2025

looks obvious. If we want to do hair splitting that's not accurate as the date when you call the function date and not when the scan is started 😃 . It's actually more or less shortly before the files are created.

Two points are still open: lets_roll() doesn't have a declaration with local . Then the current var is filled with a value in the middle of the nowhere. Either we use a date stamp when the variable is defined -- which is fine and provides better readability -- or shortly before it's being used. See comment

@drwetter drwetter mentioned this pull request Sep 30, 2025
13 tasks
@drwetter
Copy link
Collaborator

Merged this into another PR (#2904), thus closing.

@drwetter drwetter closed this Sep 30, 2025
@jdvorak001
Copy link
Contributor Author

Ok, great you had some use for this. I was laid down by a disease, unable to follow.

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.

[BUG / possible BUG] Irregular naming of output files

2 participants