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

Documentation for lease file syntax #74

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ytobi
Copy link

@ytobi ytobi commented Apr 10, 2018

Added documentation for DHCPv4/DHCPv6 lease fields, It solves ticket #5164

@vrisk
Copy link
Collaborator

vrisk commented Apr 10, 2018

Hey, Thank you for submitting a documentation improvement! I can't review the contents myself, but I am really glad to see a doc contribution.

@nchaigne
Copy link
Contributor

Is there a way to view this as HTML ?

@andrei-pavel
Copy link
Collaborator

If accepted, it will appear at https://jenkins.isc.org/job/Kea_doc/guide/kea-guide.html

Otherwise, you can clone this pull request and do:

autoreconf -i
./configure --enable-generate-docs
cd doc
make html

And you'll find it under kea-guide.html

@ytobi
Copy link
Author

ytobi commented Apr 11, 2018

@nchaigne Follow the steps outline by @andrei-pavel, I understand from the doc/ directory, you need to make guide or from the doc/guide/ you make kea-guide.html Open kea-guide.html on your bowser to view.

@nchaigne
Copy link
Contributor

Mmh... I'm having trouble building the guide. :/
(and not much time to look into this)

I/O error : Attempt to load network entity http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl
warning: failed to load external entity "http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl"
cannot parse http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl
make: *** [kea-guide.html] Error 4

@andrei-pavel
Copy link
Collaborator

Documentation 3.3 Building Requirements

The documentation generation tools elinks, docbook-xsl, libxslt and Doxygen, if using the --enable-generate-docs configuration option to create the documentation.

It basically says what you need to install, specifically docbook-xsl in this case.
For Debian-based OSes: sudo apt install docbook-xsl
For Arch-based OSes: yaourt -S --aur --noconfirm docbook-xsl
and so on...

Try it and report back?

@nchaigne
Copy link
Contributor

Thanks. (The "configure" does not check for docbook-xsl, it would be better if it did!)

Now I have other issues though:

Don't know what gentext to create for xref to: "section", ("read-only-database-configuration4")
Don't know what gentext to create for xref to: "section", ("read-only-database-configuration6")
Don't know what gentext to create for xref to: "section", ("dhcp4-ctrl-channel")
Don't know what gentext to create for xref to: "section", ("dhcp6-ctrl-channel")
Element chapter in namespace 'http://docbook.org/ns/docbook' encountered in book, but no template matches.
Element title in namespace 'http://docbook.org/ns/docbook' encountered in chapter, but no template matches.
(...)

(etc, many such lines "no template matches")

@nchaigne
Copy link
Contributor

Maybe what I did is not correct? I tried to start from Kea 1.3 tarball and patched the modified xml files.

@nchaigne
Copy link
Contributor

Alright, so I can't read the formatted documentation, but I can still review the xml you've modified. :)

I do appreciate the effort, but I feel this pull request cannot be merged in its current state.
Let me explain why (and please keep in mind this is just my opinion. I'm not speaking on behalf of ISC - they may disagree with me).

There are many typos which need to be corrected ("assinged", "clien_id", "arquire", etc.) ; one section is commented out... it sounds like you did not double-check your work.

You put two unrelated commits in the same pull request. You should have done two distinct pull requests, so maintainers have an easier job merging them. (tip: before starting on a pull request, always create a branch where you will make your changes, that way you can work on different topics at the same time)

And I still don't know how the last field (state) works, which is what prompted me to create the ticket in the first place... looking at the lease file I see "0", which I assume is "active". Maybe "1" is expired, "2" is released ? that's the kind of information I wanted to see.

@ytobi
Copy link
Author

ytobi commented Apr 22, 2018

Thanks for the review @nchaigne , I will double check, I'll fix the 1 and 3 you raised, for 2 I should have created the branch to the project here instead than in my fork.

@tomaszmrugalski tomaszmrugalski changed the title Trac5164 Documentation for lease file syntax Apr 24, 2020
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.

4 participants