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

Remove include directives (#9674) #99

Merged
merged 2 commits into from
Oct 1, 2012
Merged

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Sep 27, 2012

Include directives were introduced when maintaining a single source file for UNIX & Windows server installation.

Now instructions have been split again, it does not make sense anymore and adds confusion.

@ghost
Copy link

ghost commented Sep 28, 2012

While I don't see anything wrong in the above changes, what was wrong with the use of include directives here?

@sbesson
Copy link
Member Author

sbesson commented Sep 28, 2012

@rleigh-dundee :

  • directives were originally used to include OS-specific paragraphs. Since the whole page is now OS-specific, there is no valid justification for including these paragraphs rather than including whole subsections for instance.
  • Sphinx does a poor job with include directives compared to LaTeX for instance. Included files need to have a different extensions and are not really tracked at the top-level build.

For these reasons, I would be inclined to get rid of all include directives in the documentation for now until we decide on a valid policy about how and where to use them.

@joshmoore
Copy link
Member

@rleigh-dundee, any other objections or suggestions? Would you move each of these sections into a separate section with a toctree?

Assuming Roger accepts this for the moment, the output looks trivially correct and ready to merge.

@ghost
Copy link

ghost commented Sep 28, 2012

No objections.

joshmoore added a commit that referenced this pull request Oct 1, 2012
Remove include directives (#9674)
@joshmoore joshmoore merged commit 333dac8 into ome:master Oct 1, 2012
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.

None yet

2 participants