Skip to content

Docs for deploy generator and review of existing info - #1262

Merged
lasote merged 12 commits into
conan-io:developfrom
danimtb:feature/conan/4847
May 6, 2019
Merged

lasote merged 12 commits into
conan-io:developfrom
danimtb:feature/conan/4847

Conversation

@danimtb

@danimtb danimtb commented Apr 29, 2019

Copy link
Copy Markdown
Member

Docs for PR conan-io/conan#4972
Issue conan-io/conan#4847

  • Added section for generator class reference
  • Removed duplicated info in how-to about generator packages
  • Updated custom integration in the integration section

@danimtb danimtb added this to the 1.14 milestone Apr 29, 2019
@ghost ghost assigned danimtb Apr 29, 2019
@ghost ghost added the stage: review label Apr 29, 2019
Comment thread integrations/other.rst Outdated
@memsharded memsharded modified the milestones: 1.14, 1.15 Apr 30, 2019
Comment thread integrations/other.rst Outdated

@jgsogo jgsogo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would move the reference/generators/custom to mastering/custom_generator as it is not a reference about something that exists in Conan but something you can build on top of its functionalities.

Also, I would move the "Using template files for custom generators" section from howtos/custom_generators to mastering/custom_generator

Comment thread howtos/custom_generators.rst Outdated
Comment thread howtos/custom_generators.rst
Comment thread howtos/custom_generators.rst Outdated
Comment thread howtos/custom_generators.rst Outdated
Comment thread howtos/custom_generators.rst
Comment thread integrations/other.rst Outdated
Comment thread integrations/other.rst Outdated
Comment thread integrations/other.rst
@@ -0,0 +1,91 @@
.. _custom_generator:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wouldn't add this to the list of available generators... I would put a comment below that list ("if none of these generators fits your needs, you can write a custom generator"):

image

linking to the howtos/custom_generators or integrations/other and merge all this content there, otherwise we will have three sections of the documentation talking about the same: how to write a custom generator, and it is too much from my POV.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I understand what you mean, but we have different possibilities. The section at reference/generators/custom.rst is intended to be a reference of the generator class and the available attributes and properties. So I don't like the idea of moving it out of the reference but agree that making it part of the list is probably not ideal.

The integration part is a more generic one talking about the json generator, txt one, deps_cpp_info...

An finally the implementation of a generator package is covered in a how-to. All sections are properly cross-referenced and I think it makes

Comment thread reference/generators/custom.rst Outdated
Co-Authored-By: danimtb <danimanzaneque@gmail.com>
@danimtb danimtb removed their assignment Apr 30, 2019
Co-Authored-By: danimtb <danimanzaneque@gmail.com>
@ghost ghost assigned danimtb Apr 30, 2019
@danimtb danimtb removed their assignment Apr 30, 2019
Comment thread reference/generators.rst Outdated
generators/premake
generators/make
generators/deploy
generators/custom No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I still think we should put this out of the list (in a line or in a tip/note):

Suggested change
generators/custom
If none of these generators fits your needs, you can create your own
:ref:`custom generator <custom_generator>`.

The only caveat is that we will need the :orphan: keyword inside reference/generators/custom.rst to avoid the rst error.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Still don't like the idea of having the section so hidden, but I have taken your suggestion and we can change it in the future. Thanks for the helpful reviews

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would also add an entry, linking to this, in Extending Conan. This is clearly a point of extension and should be summarized there. Can be a short summary: purpose, goal, brief description and short recipe and link to main page

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think in that case we should move the how-to to the Extending Conan section. Otherwise, there would be too many entries for generators with bits of information around the place. I will create an issue apart so this PR can be merged

@jgsogo jgsogo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fix conflicts

@lasote
lasote merged commit e685068 into conan-io:develop May 6, 2019
@ghost ghost removed the stage: review label May 6, 2019
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