Skip to content
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

Update Breeze Documentation to have WSL 2 Instructions instead of WSL 1 #9057

Merged
merged 11 commits into from
Jun 1, 2020
Merged

Update Breeze Documentation to have WSL 2 Instructions instead of WSL 1 #9057

merged 11 commits into from
Jun 1, 2020

Conversation

notatallshaw
Copy link
Contributor

@notatallshaw notatallshaw commented May 28, 2020

WSL 2 is now available on stable version of Windows 10: https://docs.microsoft.com/en-us/windows/wsl/wsl2-index

WSL 2 works with Docker Desktop on Windows with better performance: https://docs.docker.com/docker-for-windows/wsl/

After installing WSL 2 and Docker Desktop then Breeze works with no further configuration.

WSL 1 and WSL 2 are very different technology stacks with pros and cons to both. However I have not heard of any success stories of trying to use Breeze on WSL 1, I am therefore proposing we replace the WSL 1 section with a WSL 2 section seems to work almost instantly. If someone wants to have a WSL 1 section we could 2 separate instructions .

Finally I used WSL 2 and breeze to create this pull request and have added a few extra tips based on my experience.


Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Unit tests coverage for changes (not needed for documentation changes)
  • Target Github ISSUE in description if exists
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

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.
Read the Pull Request Guidelines for more information.

@boring-cyborg
Copy link

boring-cyborg bot commented May 28, 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.
  • 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://apache-airflow-slack.herokuapp.com/

BREEZE.rst Outdated Show resolved Hide resolved
BREEZE.rst Outdated Show resolved Hide resolved
BREEZE.rst Outdated Show resolved Hide resolved
@potiuk
Copy link
Member

potiuk commented May 29, 2020

Nice! Thanks! Few nits and style pre-commit check which is I think about extra spaces. FYI if you install pre-commit (see https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#pre-commit-hooks) the style will be fixed automatically during pre-commit :)

BREEZE.rst Outdated Show resolved Hide resolved
BREEZE.rst Outdated Show resolved Hide resolved
BREEZE.rst Show resolved Hide resolved
@notatallshaw
Copy link
Contributor Author

Thanks for your time, I'll take a look over all of these and fix them.

notatallshaw and others added 4 commits May 30, 2020 14:00
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Co-authored-by: Felix Uellendall <feluelle@users.noreply.github.com>
Co-authored-by: Felix Uellendall <feluelle@users.noreply.github.com>
@potiuk
Copy link
Member

potiuk commented May 30, 2020

Still some spaces at the end of lines :(.

@notatallshaw
Copy link
Contributor Author

Still some spaces at the end of lines :(.

Still working on it, got a spell check extension and fixed a spelling issue, just figured out how to set-up pre-commit and now latest commit passes pre-commit tests. Moving on to linting next to see if there's some formatting I'm missing.

Thanks for you patience, open source contribution and tooling is new to me.

@potiuk
Copy link
Member

potiuk commented Jun 1, 2020

Perfect! Thank you for your patience as well :). We do have quite a set of tools that make our work really consistent even for newcomers and make committer lives a bit easier as we automate all checks we can :).

@potiuk potiuk merged commit a85d228 into apache:master Jun 1, 2020
@boring-cyborg
Copy link

boring-cyborg bot commented Jun 1, 2020

Awesome work, congrats on your first merged pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants