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

1908/add description of split at sign #1946

Merged
merged 3 commits into from Nov 28, 2019

Conversation

@plettich
Copy link
Member

plettich commented Nov 28, 2019

No description provided.

plettich added 2 commits Nov 18, 2019
- improve documentation for `splitAtSign` configuration
- add some links
- fix some warnings

Fixes #1908
@plettich plettich requested a review from cornelinux Nov 28, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 28, 2019

Codecov Report

Merging #1946 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1946   +/-   ##
=======================================
  Coverage   97.21%   97.21%           
=======================================
  Files         153      153           
  Lines       18555    18555           
=======================================
  Hits        18039    18039           
  Misses        516      516

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3abe7f5...218ab94. Read the comment docs.

Copy link
Member

cornelinux left a comment

Understanding tables and graphs always needs a bit more input ;-)

:ref:`split<splitatsign>` realm.

The following table shows different combinations of a users login and a *realm*
parameter and where the user will be looked for depending on the

This comment has been minimized.

Copy link
@cornelinux

cornelinux Nov 28, 2019

Member

Should it be "where a user will be looked up" or "where the user will be searched"?

:ref:`splitatsign` setting:

============= ======= ======================= =====
Input :ref:`splitatsign`

This comment has been minimized.

Copy link
@cornelinux

cornelinux Nov 28, 2019

Member

I was thinking a bit, what I actually see in this table.
Could you either add some explanation or enhance the headers:

Could you change "Input" to "Input parameter", so that it is clear, that these are request parameters.

Hm, the login is actually the user parameter (or username). I am not sure, if we should change this or if "login" is fine.

Could you change the header :ref:`splitatsign` to splitatsign-setting (so that it is clearer, that this is the actual configuration)

Also add unicode arrow for better readability but this needs either
non-breakable-white-spaces (nbsp) or CSS. I added a custom CSS here to
avoid cluttering the docs.
@cornelinux cornelinux merged commit 7abf658 into master Nov 28, 2019
5 checks passed
5 checks passed
ci/circleci Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 3abe7f5...218ab94
Details
codecov/project 97.21% remains the same compared to 3abe7f5
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@cornelinux cornelinux deleted the 1908/add_description_of_splitAtSign branch Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.