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

Fix NGINX related documentation #168

Merged
merged 2 commits into from Mar 22, 2017

Conversation

Projects
None yet
3 participants
@easybe
Contributor

easybe commented Mar 19, 2017

When logging multiple virtual hosts we need to use the
common_complete log format it is defined as
host + ncsa_extended.

easybe added some commits Mar 19, 2017

Fix NGINX related documentation
When logging multiple virtual hosts we need to use the
`common_complete` log format it is defined as
`host + ncsa_extended`.
@mattab

This comment has been minimized.

Member

mattab commented Mar 21, 2017

Hello @easybe

We used to have this in the documentation but @cweiske changed our doc in #154 6 months ago.

Please @cweiske can you discuss with @easybe and advise us what is the correct documentation or how we can clarify things better?

@cweiske

This comment has been minimized.

Contributor

cweiske commented Mar 22, 2017

I guess both need to be listed, with a which to use when using multiple hosts and a single one.

@easybe

This comment has been minimized.

Contributor

easybe commented Mar 22, 2017

If you read the paragraph "Example Nginx Virtual Host Log Format" carefully you should see that there must be a copy & paste mistake. The first line tells you what format name to use with NGINX's default logging settings. But the last line should tell you which format to use in a virtual host setup. Christian Weiske noticed that --log-format-name=common_complete doesn't work with NGNIX's default settings. This makes sense because in the source code we see that common_complete is defined as _HOST_PREFIX + _NCSA_EXTENDED_LOG_FORMAT. He shouldn't have changed the second occurrence of --log-format-name=common_complete though.

@cweiske

This comment has been minimized.

Contributor

cweiske commented Mar 22, 2017

@easybe you are right. The second should be reverted.

@mattab

This comment has been minimized.

Member

mattab commented Mar 22, 2017

Thanks for confirming guys, and for keeping our docs accurate 👍

@mattab mattab merged commit bd60a20 into matomo-org:master Mar 22, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment