Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add docu for deployers and a simple example #2946
Add docu for deployers and a simple example #2946
Changes from 3 commits
5f8e71e
8220988
43c14b1
d71b4bb
5f5c892
721372a
c6f43d3
72ed52a
0aaecd0
1b1363f
9d501ef
12fac7a
80ed7f1
26da31a
f3a31e1
21f66b4
faf1365
27448c7
f774c5e
81c0993
125bba4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get:
is it expected? or typo in path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is expected as the PR in examples2 has not yet been merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dependencies_sources
might be better?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this
dependency_sources
folder located?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, as it's for conan 2 and python 3+, it may already use
Pathlib
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better not document a private object yet (nothing in the
conans
namespace should be in the docs at all), because users might thing they can instantiate or use their methods. Until we document the API of the graph, better treat it as opaque as possible, just saying that it has aroot.conanfile
would be enough.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if consumer receives some sort of opaque object, they need to know that are the valid methods/attributes to be safely called.
if object itself cannot be documented, then it should define some sort of an interface.
e.g. how do I know that
graph
hasgraph.root
, andgraph.root
hasgraph.root.conanfile
, andgraph.root.conanfile
hasgraph.root.conanfile.dependencies
that is some sort of dict.how do I know that are these object and are they stable, if they are never explained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, it is enough to say that
graph.root.conanfile
is available, and that has the documenteddependencies
attribute, which is good for most cases. Let's just document that, but not the whole object private interface.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and documentation on the dependencies interface says explicitly:
This information does not exist in some recipe methods, only in those methods that evaluate after the full dependency graph has been computed.
,At the moment, this information should only be used in generate() and validate() methods. Any other use, please submit a Github issue.
. it doesn't seem to say it's available indeploy()
method and allowed to be used there. that's a bit contradictory from the documentation to say in one place it's not allowed, and in another it's allowed 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are referencing Conan 1.X docs. The docs for 2.0 will be (and already are) very different to those of 1.X. Deployers is a 2.0 feature, and this PR is for 2.0-beta
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it says the same in 2.0 docs: https://docs.conan.io/en/2.0/reference/conanfile/other/dependencies.html?highlight=dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it says the same in 2.0 docs: https://docs.conan.io/en/2.0/reference/conanfile/other/dependencies.html?highlight=dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, yes, that would be a bug in the docs, seems it was a copy&paste from 1.X
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do they need to affect on generators behavior? that's super confusing part.
as a consumer, I'd expect deployers just to deploy (copy) things, and otherwise, leave other parts untouched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is one of most requested behaviors for deployers and generators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's kinda surprising behavior that deployers may modify files generated by generators. can it be made more explicit? e.g.
--deploy=full_deploy --deploy-modify-generator=CMakeDeps
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is kinda unspecified, what happens in case of multiple deployers, if all of them want to change target directories, what happens?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, the last one would win