-
Notifications
You must be signed in to change notification settings - Fork 906
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/re structure config docs #2421
Conversation
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
# Conflicts: # docs/source/kedro_project_setup/configuration.md # docs/source/nodes_and_pipelines/run_a_pipeline.md
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
|
Discussed synchronously between @merelcht, @stichbury and I:
|
…ween explanations and how tos Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
…ds and params Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
@stichbury @astrojuanlu I've made a first pass of dividing the configuration docs into basic and advanced chapters. I think the basics are in pretty good shape now. Can you have a look and tell me what you think? (ignore the links I haven't fixed them yet in case the pages still change) You can ignore the advanced page for now. I also need to decide where the sections on |
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
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 really like the direction this is taking! 💯
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
I've made a number of commits to answer some of the queries/suggestions you'd left above (and some notes above to summarise). I think there's probably a bit left for you, Merel, before we hand over to @juan Luis for a final review (or you can pass it back to me to make another round of changes). It's looking SO. MUCH. BETTER. Thank you 🙏
|
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
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.
Read the whole thing, fantastic job @merelcht 👏🏽 I did learn a lot! Left a few style comments but other than that this is a great addition
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
# Conflicts: # docs/source/kedro_project_setup/configuration.md
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
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.
+1 on @stichbury's comment about providing links in the basic docs to related sections in the advanced config page that might be of interest.
Read through the docs, it looks great! 💯
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
I fixed all the links and linked the individual config pages to each other where it made sense. I guess over to you one more time @stichbury / @astrojuanlu for a final review. |
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'm really happy with how this turned out -- thanks @merelcht! 🏆
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com> Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Description
Development notes
credentials
andparameters
on two separate pages.Remaining tasks/decisions to make
TemplatedConfigLoader
still has quite a lot of content with the "Provide template values through globals" section, but I feel like if we move that to the "how to.." section it's not clear at all what this loader is about or why you'd use it. Any suggestions on how to make the intro to theTemplatedConfigLoader
shorter but still clear how to do the templating? JCS: I feel like this section is not really explaining "why" to the reader. So it starts as follows:But what is the use case by which I'd need this? (I can't answer that, sorry). Would it be possible to turn this into more of a "how to" by adding a sentence or two that explains the motivation that a user would have in using a
globals
file? So retitling from "Provide template values through globals" to something like "How to provide a dynamic set of settings using global values" (totally made up). Then give a sentence or two as suggested and launching into the details? I still think it's fine where it is, but you can make it more useful by positioning it better.credentials
andparameters
be under the basic or advanced configuration or on separate pages as I've done now? : JCS: I like the separate pagesChecklist
RELEASE.md
file