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

doc: Add PHP section to index #86401

Merged
merged 3 commits into from May 3, 2020
Merged

doc: Add PHP section to index #86401

merged 3 commits into from May 3, 2020

Conversation

etu
Copy link
Contributor

@etu etu commented Apr 30, 2020

Motivation for this change

I guess this should work? We have the markdown file from before and I guess that is converted to xml somehow?

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach is right since the .md-files are converted to docbook using pandoc first.

However, nix-build doc/ is breaking with this change due to a XML-id collision: python.section.md and php.section.md have a User guide-headline which is transformed into an xml:id named user-guide (twice).

It may be possible to work around this in pandoc (in doc/Makefile) using --id-prefix, the easier fix would be however to replace the User guide-headline in php.section.md by something else I guess.

@etu
Copy link
Contributor Author

etu commented May 1, 2020

TIL how to build the documentation locally!

I have made some changes to our headlines and tested it locally :-)

@flokli
Copy link
Contributor

flokli commented May 1, 2020

I tried updating doc/Makefile to use --id-prefix, but unfortunately, this also prefixes ALL links in the markdown file, breaking references from .md to somewhere outside.

I'll ask upstream pandoc if there's a way around this, but I assume for now we can't namespace .md documentation, and need to be super careful with how we name our headings :-/

@etu
Copy link
Contributor Author

etu commented May 1, 2020

I'll ask upstream pandoc if there's a way around this, but I assume for now we can't namespace .md documentation, and need to be super careful with how we name our headings :-/

That means that we shouldn't make headings like Overview because someone else would probably want to have that as well. But prefixing with something like $lang overview should be fairly safe since it's kinda namespaced to one of the files.

doc/languages-frameworks/php.section.md Outdated Show resolved Hide resolved
doc/languages-frameworks/php.section.md Outdated Show resolved Hide resolved
@talyz
Copy link
Contributor

talyz commented May 2, 2020

After looking at the rendered documentation a bit closer, I think it would be good to just get rid of the User Guide heading, since there's no other header at the same level anyway. What do you think?

@etu
Copy link
Contributor Author

etu commented May 2, 2020

My thought is that it can be good to keep, the reasoning behind that is that I in the future would like to see that we manage to get something in to build things with composer.

And that would be it's own headline among the lines Developing guide on the same level as User guide.

But I already dropped one level in this PR 😄

@talyz
Copy link
Contributor

talyz commented May 2, 2020

Okay, yeah, that makes sense. I guess we could also document buildPecl and such, which should probably be under a different heading on that level. :)

@etu etu merged commit 776f12b into NixOS:master May 3, 2020
@etu etu deleted the php-add-doc-to-index branch May 3, 2020 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants