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

Docs for deploy generator and review of existing info #1262

Merged
merged 12 commits into from May 6, 2019

Conversation

danimtb
Copy link
Member

@danimtb danimtb commented Apr 29, 2019

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
integrations/other.rst Outdated Show resolved Hide resolved
@memsharded memsharded modified the milestones: 1.14, 1.15 Apr 30, 2019
integrations/other.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@jgsogo jgsogo left a comment

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

howtos/custom_generators.rst Outdated Show resolved Hide resolved
howtos/custom_generators.rst Show resolved Hide resolved
howtos/custom_generators.rst Outdated Show resolved Hide resolved
howtos/custom_generators.rst Outdated Show resolved Hide resolved
howtos/custom_generators.rst Show resolved Hide resolved
integrations/other.rst Outdated Show resolved Hide resolved
integrations/other.rst Outdated Show resolved Hide resolved
integrations/other.rst Show resolved Hide resolved
@@ -0,0 +1,91 @@
.. _custom_generator:
Copy link
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
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

reference/generators/custom.rst Outdated Show resolved Hide resolved
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
@@ -43,3 +43,5 @@ Available generators:
generators/json
generators/premake
generators/make
generators/deploy
generators/custom
Copy link
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
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
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
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

Copy link
Contributor

@jgsogo jgsogo left a comment

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.

None yet

4 participants