Skip to content

Update initialize-database.rst#11109

Merged
XD-DENG merged 2 commits intoapache:masterfrom
jmfreeman:patch-1
Sep 23, 2020
Merged

Update initialize-database.rst#11109
XD-DENG merged 2 commits intoapache:masterfrom
jmfreeman:patch-1

Conversation

@jmfreeman
Copy link
Copy Markdown
Contributor

Remove ambiguity in the language as only MySQL and Postgres are supported backends.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

Remove ambiguity in the language as only MySQL and Postgres are supported backends.
@boring-cyborg
Copy link
Copy Markdown

boring-cyborg bot commented Sep 23, 2020

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:

  • Pay attention to the quality of your code (flake8, pylint and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

Copy link
Copy Markdown
Member

@XD-DENG XD-DENG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQLite should be mentioned given it's important for development/testing purpose.

Almost all new users start with SQLite (e.g. to test if the installation/setting-up is working). It will be weird to exclude it here.

Edit: what I meant is to mention SQLite (and highlight it's primarily for development purpose), to ensure consistent information. Otherwise people having quick looking at this may have the confusion "SQLite is not supported".

@potiuk potiuk requested a review from XD-DENG September 23, 2020 19:27
Copy link
Copy Markdown
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with the "SQLite" modification

Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@XD-DENG
Copy link
Copy Markdown
Member

XD-DENG commented Sep 23, 2020

@potiuk I made some minor changes on top of your suggestion (add "as", and fix the trailing comma to period.)

@XD-DENG XD-DENG merged commit b83507a into apache:master Sep 23, 2020
@boring-cyborg
Copy link
Copy Markdown

boring-cyborg bot commented Sep 23, 2020

Awesome work, congrats on your first merged pull request!

@XD-DENG
Copy link
Copy Markdown
Member

XD-DENG commented Sep 23, 2020

Thanks @jmfreeman for the contribution. Hope you are ok with the changes that @potiuk and I have made.

@potiuk
Copy link
Copy Markdown
Member

potiuk commented Sep 23, 2020

Seems like one-sentence changes are those that gets the most scrutiny :) cool stuff!

@potiuk
Copy link
Copy Markdown
Member

potiuk commented Sep 23, 2020

And we have a new contributor ;) @jmfreeman ... Looking forward to more contributions !

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.

3 participants