-
-
Notifications
You must be signed in to change notification settings - Fork 228
doc: add inheritance and composition section in user docs #2165
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
base: master
Are you sure you want to change the base?
doc: add inheritance and composition section in user docs #2165
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2165 +/- ##
==========================================
- Coverage 98.03% 97.90% -0.14%
==========================================
Files 55 55
Lines 6005 6108 +103
==========================================
+ Hits 5887 5980 +93
- Misses 118 128 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ffef11b to
63b12a5
Compare
Co-authored-by: fpaterour-ippon <[email protected]>
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.
Thanks for volunteering to write documentation on this complex topic, @francesco086! 🙏 I have a few suggestions, curious what you think.
Regarding solutions: Meta-templates is another one, although I don't know how relevant it actually is in practice.
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.
How about renaming the file to reusing.md (intentionally as a gerund for consistency with some other files like configuring.md) to keep the URL path concise and also less specific to a particular reuse approach?
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.
My feeling is that reusing is too generic, and it is especially confusing in the context of copier. The first thing that would come to my mind is "Of course! I have some code that I want to re-use by creating a template! Let's see how".
What about composing, composing_templates, or inheriting_and_composing_templates (too long perhaps)? Alternatively, reusing_templates may be explicit enough.
I leave the final decision to you / the maintainers, just let me know what do you think is best and I will apply 😄
| @@ -0,0 +1,36 @@ | |||
| # Template Inheritance and Composition | |||
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.
For consistency with the renamed file:
| # Template Inheritance and Composition | |
| # Template Reuse |
| @@ -0,0 +1,36 @@ | |||
| # Template Inheritance and Composition | |||
|
|
|||
| ## What is Inheritance, what is Composition? | |||
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.
Perhaps this is more concise?
| ## What is Inheritance, what is Composition? | |
| ## Inheritance vs. Composition |
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.
True, but it miss the key point that this section is "What are we talking about", and not only comparing them.
In my head this section was the WHAT. Then there is the WHY, and finally HOW (solutions).
docs/inheritance_and_composition.md
Outdated
|
|
||
| Template Inheritance is a way to create a new template based on an existing one. It | ||
| allows you to reuse code and structure from the parent template while adding or | ||
| overriding specific parts in the child template. This is useful for creating variations |
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 it's mostly about extensions, not so much variations – perhaps a matter of definition. Variations – in my mind – can often be addressed by conditional questions and conditional rendering in the same template. WDYT?
| overriding specific parts in the child template. This is useful for creating variations | |
| overriding specific parts in the child template. This is useful for creating extensions |
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 agree with what is in your head, but this is exactly the reason why I think inheritance is useful, to avoid the ugly if/else.
By the way this is exactly what I did in my example, where we did not agree that it was inheritance.
I would like to mention that if you look carefully at the example you may notice that I did start with if/else (this is the key question), but it became a horror to maintain the template. Very complex, very hard to read, and very slow tests. This is why on the next flavor I had to add, I decided for this "inheritance" approach, which worked decently, except for the drawbacks you already clarified very well, especially when it comes to copier update.
But you made a good point, in my opinion we should mention both. An alternative term for "variation" is "flavor" by the way.
Do you think we can agree to write "This is useful for creating extensions and variations"?
docs/inheritance_and_composition.md
Outdated
| Template Composition, on the other hand, is a way to combine multiple templates, | ||
| independent of each other, into a single new template. In this way you can join the | ||
| functionalities of multiple templates, by adding some "glue code", and avoid repetition. |
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.
Do you have an approach like yours using tasks that run copier copy in mind? I've mostly thought of "composition" along the lines of https://copier.readthedocs.io/en/stable/configuring/#applying-multiple-templates-to-the-same-subproject. There, template composition means generating output from multiple templates in the same target directory, but there is no new template that is a composition of other templates.
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 it is up to discussion!
The solution you mentioned is what I consider the "manual" solution. But of course leaves the problem of "what if it is very common to compose always the same 3 templates" and want to make it more convenient for a user to get the 3 together and don't need to manually check that they have compatible versions?
But yes, I think that using tasks could be a way, of course with the drawbacks already mentioned.
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.
To avoid forgetting about it, I also added this to the solutions
| - Generating a project: "generating.md" | ||
| - Updating a project: "updating.md" | ||
| - Settings: "settings.md" | ||
| - Template Inheritance and Compostion: "inheritance_and_composition.md" |
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.
See above:
| - Template Inheritance and Compostion: "inheritance_and_composition.md" | |
| - Reusing templates: "reusing.md" |
docs/inheritance_and_composition.md
Outdated
| ## Why copier does not natively support inheritance and composition? | ||
|
|
||
| <!-- TODO: add content, re-use content from https://github.com/copier-org/copier/issues/934#issuecomment-1518964035 --> |
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 anticipate that a proper solution might require Copier to support it natively. At least for the fork-like workflow, I think that specified template versions in parent template migrations need to be mapped to child template versions – at the very least. But it's too early to make a definitive statement.
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 see that coming too! But I think @yajo had some very valid points. So, let's see where do we end up to, and let's try to keep what he said in mind.
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.
Reading carefully what he wrote once again, it seems to me that everything reduces to the UNIX philosophy of "do one thing well", or am I wrong?
I filled the WHY section
Right! I think it is not really a solution for inheritance and composition tbh, but I added it to the list, and we will discuss it at due time |
Co-authored-by: Sigurd Spieckermann <[email protected]>
Co-authored-by: Sigurd Spieckermann <[email protected]>
Co-authored-by: Sigurd Spieckermann <[email protected]>
|
@sisp I am back from vacation and worked on the documentation last week and I just finished a demo of the approach I was suggesting using tasks and migrations. You can find it here: https://github.com/francesco086/copier-inheritance-via-task-and-migration-demo-child I will reference it in the documentation and keep moving with other approaches 😄 |
|
Thanks, @francesco086! I'll take a look next week, out of office myself at the moment. 😉 |
Enjoy your time off!!! |
65fde0e to
9f1e131
Compare
9f1e131 to
241e4ff
Compare
Co-authored-by: Sigurd Spieckermann <[email protected]>
|
Hi @sisp , I am coming to this after quite some long time. I was trying to look at the alternative ideas, but to me they all look way more complicated and with relevant drawbacks compared to the one I already documented. Could you perhaps have a look at what I have done and let me know if I should still try to document different approaches? |
|
Thanks for the reminder, @francesco086! 👍 I definitely see some advantages of the task/migrations-based approach, especially the encapsulation of the parent template. But I struggle with the implementation of a simple scenario like this: Imagine you have a generic Python project template (parent) and a Python FastAPI template (child). Consider, e.g., the |
|
Hey @sisp ! Yes, you are absolutely right. One would make a copy-paste (or git sparse clone) from the parent, and modify it. The real problem would come with the parents upgrade (notice that since the parent version is pinned, this is under manual control). You would have to do the same procedure mentioned before. You could use diff tools to help yourself, but eventually it would be a manual process. Frankly I don't think one can do much better. In these scenarios usually git gives you conflicts to resolve manually. But I may be mistaken. In general, I think one should try to avoid the overwriting of a file, so it should be limited to as few files as possible. For example, in the use case you mentioned, one could use dynamic dependencies and only overwrite a requirements file (of course this requires being able to influence the parent template, which is not always granted) This to say that I think that this drawback has not such strong impact. |
|
Thanks for confirming. This means, the limitations of task/migrations-based template inheritance impact design decisions of the generated project. 🤔 The dependencies example is only one among many. For example, a generic Python project template may have a basic pytest configuration, and a FastAPI template may extend the starter to include some E2E tests setup which requires additions in [tool.pytest.ini_options]
...
-addopts = "-n auto"
+addopts = "-n auto --strict-markers -m 'not e2e'"
+markers = ["e2e: mark tests as E2E tests"]
...It feels like these kinds of surgical edits are unpredictable. Of course, I'm biased towards the "fork"-based approach, but it does support these scenarios – at the cost of a fully materialized parent template and a need to sync the "fork" with its upstream template. We're exploring this approach internally, so far it's looking fine, but we'll need to gain more experience over time. That said, the task/migrations-based approach may work just fine with less complexity for some template hierarchies that don't need surgical edits. |
|
Do you have a reference of this fork-like approach for me to document it? It could be my next method to document. I will also add this limitation you pointed out to the cons. |
Contributes to #934