Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename logfile to minetest.log by default #8434

Closed
wants to merge 1 commit into from

Conversation

ClobberXD
Copy link
Contributor

@ClobberXD ClobberXD commented Mar 31, 2019

Changes

  • Make logfile name configurable. Relevant minetest.conf setting: logfile_name.
  • logfile_name defaults to minetest.log.
  • Replace all hard-coded occurrences of debug.txt in the code with the actual logfile name
  • Report selected logfile name (incl. full path) to terminal on startup.

A couple of reasons for the proposed changes

  • Customizable logfile name is a fundamental feature, IMHO.
  • The default logfile name of debug.txt makes Minetest sound more like a beta game, which it isn't. :)
  • The proposed default logfile name makes Minetest sound more polished. (This is basically an extension of the previous point)
  • There are so many other things that are logged in that file other than just warnings and errors.
  • The .log file extension is the de-facto standard for log files.

@ClobberXD ClobberXD force-pushed the rename_logfile branch 2 times, most recently from 36a851a to c449bde Compare March 31, 2019 10:56
@paramat
Copy link
Contributor

paramat commented Mar 31, 2019

The .log file extension is the de-facto standard for log files.

It doesn't matter what the standard is, let's just do what's best for MT.
'minetest.log' sounds ok to me, consistent with 'minetest.conf'.

@benrob0329
Copy link
Member

Things tend to be standard for a reason. If most software behaves in a certain way, its usually because its sane behavior that users expect.

@paramat
Copy link
Contributor

paramat commented Apr 1, 2019

I mostly agree and i didn't state otherwise. It's useful to consider the standard as it may be a good idea, but it's not necessarily right for MT.

@rubenwardy
Copy link
Member

I don't think that the benefits of this are worth changing all the documentation and any bash scripts that rely on this

@ClobberXD
Copy link
Contributor Author

ClobberXD commented Apr 4, 2019

I don't think that the benefits of this are worth changing all the documentation and any bash scripts that rely on this

That's not a very strong reason to support sticking to this name. I'll add a setting to make logfile name configurable - this is something I'd consider essential, even without changing the default name.

@ClobberXD
Copy link
Contributor Author

Added a new setting for customizing logfile name - logfile_name (duh :P). See the updated first post for current status of this PR.

…name

Also:
- Replace all hardcoded occurences of logfile name in the code with configured logfile name
- Report selected logfile to terminal on startup
- Add pattern minetest.log to .gitignore
@rubenwardy
Copy link
Member

rubenwardy commented Jun 23, 2019

We really don't need more configuration options. This would make more sense as a command line argument
EDIT: which already exists, as --logfile

@ClobberXD
Copy link
Contributor Author

Also, look how many times debug.txt appears in random deployment things

Should I retain the default name then?

This would make more sense as a command line argument

We already have a --logfile switch.

Keeping these points in mind, what should be done now?

@paramat
Copy link
Contributor

paramat commented Jun 23, 2019

I agree with rubenwardy and SmallJoker (emoticon support).
And a setting is certainly overkill.
👎

@paramat paramat closed this Jun 23, 2019
@paramat paramat added the Won't add The feature request was rejected and will not be added. label Jun 23, 2019
@ClobberXD ClobberXD deleted the rename_logfile branch June 24, 2019 02:48
@ClobberXD ClobberXD restored the rename_logfile branch August 9, 2019 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Won't add The feature request was rejected and will not be added.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants