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

Add default value of parameters to parameter description in config.sample.php and the documentation #3069

Closed
j-ed opened this issue Jan 13, 2017 · 6 comments
Labels
1. to develop Accepted and waiting to be taken care of enhancement good first issue Small tasks with clear documentation about how and in which place you need to fix things in. pending documentation This pull request needs an associated documentation update

Comments

@j-ed
Copy link
Contributor

j-ed commented Jan 13, 2017

Expected behavior

For all configuration parameters in config.sample.php and in the documentation an information should be available what the default value of a parameter is. This would allow an administrator to reduce the size of a configuration file to a minimum because only parameters would need to be set which differ from a default value.
The easiest way to recognize a default value is, by listing it at the end of a parameter description, prefixed by e.g. the wording Default to ....

Current behavior

In several posting on GitHub and on the Internet I read that someone asked "Why have you added this and that parameter to the configuration, it is the default value?" The reason seems to be, that information is missing for most of the listed parameters in config.sample.php and in the documentation, what the default value of a parameter is.

Negative example:

/**
 * When enabled, admins may install apps from the Nextcloud app store.
 */
'appstoreenabled' => true,

Positive example:

/**
 * Log file path for the Nextcloud logging type.
 * Defaults to ``[datadirectory]/nextcloud.log``
 */
'logfile' => '/var/log/nextcloud.log',

Steps to reproduce

  1. Check the config.sample.php and the documentation.

Environment

Server Configuration

OS: Linux 3.2.82
Web server: Apache2 2.4.25
Database: MariaDB 5.5.53
PHP version: 5.6.29
Nextcloud version: 10.0.2

Client Configuration

Browser: Mozilla Firefox 50.1.0
Operating system: Windows 7

@nickvergessen nickvergessen added this to the Nextcloud 12.0 milestone Jan 13, 2017
@nickvergessen nickvergessen added enhancement good first issue Small tasks with clear documentation about how and in which place you need to fix things in. labels Jan 13, 2017
@j-ed
Copy link
Contributor Author

j-ed commented Jan 13, 2017

@nickvergessen I could try to add this information to the config.sample.php file based on information from the source code, but I need to get the following questions answered before I start:

  • Who can I contact if I need to ask a question concerning a specific parameter?
  • What is the default line length in the config.sample.php file, less than 75 or 80 characters?
  • How should I address contradictory default values in the source code? Should I address all my findings in one issue ticket or do I need to create separate ones for each parameter?
  • How should I address supposed missing default value assignments in the source code?
  • How should I address different writings of default value assignments in the source code? I'm not a professional software developer to judge if the found writing is always the same.

@nickvergessen
Copy link
Member

Who can I contact if I need to ask a question concerning a specific parameter?

Me

What is the default line length in the config.sample.php file, less than 75 or 80 characters?

should be 80 chars

How should I address contradictory default values in the source code? Should I address all my findings in one issue ticket or do I need to create separate ones for each parameter?

You mean one parameter with 2 different default values? Please create an issue for each of them

How should I address supposed missing default value assignments in the source code?

Just point them out, I will then comment on them

How should I address different writings of default value assignments in the source code? I'm not a professional software developer to judge if the found writing is always the same.

Just ask if you find such cases

@j-ed
Copy link
Contributor Author

j-ed commented Jan 16, 2017

@nickvergessen Should I open an issue ticket for variables that are using none and null too, to get the default values aligned? Example:

+ getSystemValue('memcache.local', 'none')
- /var/www/htdocs/nextcloud/apps/survey_client/lib/Categories/Server.php:72
- /var/www/htdocs/nextcloud/apps/serverinfo/lib/SystemStatistics.php:53

+ getSystemValue('memcache.local', null)
- /var/www/htdocs/nextcloud/settings/Controller/CheckSetupController.php:138
- /var/www/htdocs/nextcloud/lib/private/Server.php:381

@nickvergessen
Copy link
Member

Hmm looking at a couple of issues, it seems that default doesn't really have to be the same value. E.g. the getSystemValue('memcache.local', 'none') from your previous comment, they are not actually setting up a memcache, but just want to use the string "none" if you didn't explicitly set it to any other caching.

@j-ed
Copy link
Contributor Author

j-ed commented Jan 16, 2017

As far as I understood none is a fixed value assignment and null means undefined.
So if the same parameter is read but different default values are assigned in the source code I would assume possible problems in the future. That's the reason why I asked for it.

@nextcloud-bot nextcloud-bot added the stale Ticket or PR with no recent activity label Jun 20, 2018
@nextcloud-bot nextcloud-bot removed the stale Ticket or PR with no recent activity label Oct 1, 2018
@skjnldsv skjnldsv added 0. Needs triage Pending check for reproducibility or if it fits our roadmap 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Aug 20, 2020
@joshtrichards joshtrichards added the pending documentation This pull request needs an associated documentation update label Jan 31, 2024
@joshtrichards
Copy link
Member

Fixed in #3133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of enhancement good first issue Small tasks with clear documentation about how and in which place you need to fix things in. pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants