RNHTTR commented on PR #23762:
URL: https://github.com/apache/airflow/pull/23762#issuecomment-1130521006

   A few thoughts:
   1. I think it'd make sense to mention Breeze in 
[CONTRIBUTORS_QUICK_START.rst](https://github.com/apache/airflow/blob/bc6ad899adce71fd39ee01206c169312fe86fe4c/CONTRIBUTORS_QUICK_START.rst).
 Maybe just a high level overview and a link to 
[BREEZE.rst](https://github.com/apache/airflow/blob/bc6ad899adce71fd39ee01206c169312fe86fe4c/BREEZE.rst).
 Perhaps in the "Note to Starters" or "Local machine development" sections?
   2. My concern would be how maintainable will these guides be? If something 
common to each of the guides changes, they're not very DRY, and the update will 
have to be made in each sub-guide. Maybe common parts like "Forking and cloning 
Project", "Setting up Breeze", "Installing airflow in the local virtual 
environment airflow-env with breeze.", "Using Breeze", "Testing", etc could be 
included in the "main" 
[CONTRIBUTORS_QUICK_START.rst](https://github.com/apache/airflow/blob/bc6ad899adce71fd39ee01206c169312fe86fe4c/CONTRIBUTORS_QUICK_START.rst).
 Perhaps there could be a link at the end of each sub-guide that returns to 
`CONTRIBUTORS_QUICK_START.rst` to finish the guide with these sections.
   3. Nitpick, but in 
[CONTRIBUTORS_QUICK_START.rst](https://github.com/apache/airflow/blob/bc6ad899adce71fd39ee01206c169312fe86fe4c/CONTRIBUTORS_QUICK_START.rst),
 maybe it should say "Contributor's Quick Start" or "Contributor's Quick Start 
Guide" instead of "Contributor's Quick Guide".
   4. Another nitpick: IMO 
[CONTRIBUTORS_QUICK_START_VSC.rst](https://github.com/apache/airflow/blob/bc6ad899adce71fd39ee01206c169312fe86fe4c/CONTRIBUTORS_QUICK_START_VSC.rst)
 should be named `CONTRIBUTORS_QUICK_START_VSCODE.rst`.
   5. Yet another nitpick: Each of the sub-guides starts with "Setup Airflow 
with Breeze" and "Forking and cloning Project" as a header. Maybe these headers 
should be removed; I'm not sure there should be a header for Breeze here 
because it's explained later in the guides, and there's no alternative to 
Breeze given. Does that make sense? Similarly, maybe the "Forking and cloning 
Project" header should be removed because it's described in step 1 and there's 
no alternative.
   
   Overall I think this is much better! Let me know what you think.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to