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

[AIRFLOW-XXX] Move out the examples from integration.rst #4672

Merged
merged 1 commit into from
Feb 8, 2019

Conversation

mik-laj
Copy link
Member

@mik-laj mik-laj commented Feb 8, 2019

Make sure you have checked all steps below.

Jira

No applicable

Description

Currently, integration.rst file is a list of operators grouped by services and cloud provider. Examples should not be included there. I'm moving example to another place. This is not the best place. Such things should be in the howto/operator.rst file, but this would require writing a lot more documentation. Other operators have examples in the class description, so this is not an invalid change. Step by step. Cleaning integration.rst file is the first goal. In the future, I hope that we will be able to improve class descriptions.

Tests

No applicable

Commits

  • My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

No applicable

Code Quality

No applicable

@codecov-io
Copy link

Codecov Report

Merging #4672 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4672   +/-   ##
=======================================
  Coverage   74.35%   74.35%           
=======================================
  Files         428      428           
  Lines       27947    27947           
=======================================
  Hits        20779    20779           
  Misses       7168     7168
Impacted Files Coverage Δ
airflow/contrib/operators/dataflow_operator.py 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e607106...b256be5. Read the comment docs.

@mik-laj
Copy link
Member Author

mik-laj commented Feb 8, 2019

@ashb Can you look at it?

@feng-tao
Copy link
Member

feng-tao commented Feb 8, 2019

lgtm, so now all the example should be put in the docstring for the operator/hook instead of doc? Maybe clarify this somewhere in the doc if you haven't.

@feng-tao feng-tao merged commit d11147a into apache:master Feb 8, 2019
@mik-laj
Copy link
Member Author

mik-laj commented Feb 8, 2019

Currently, examples can be in two places - docstring for classes and howto/operator.rst file. If it's just simple an example code, it's good when it's in the class description. If a longer description is needed, then I think that the howto / operaotr.rst file is a better place. In the future, however, we should choose only one place and merge contents.

I note that both places are connected with links: #4671

I do not think that an additional explanation is necessary. When someone opens this integration.rst file, the purpose of this file is legible for him. I prepared a preview for you: http://glistening-theory.surge.sh/integration.html
I cleaned the integration.rst file from unwanted things earlier.

I think that before unification operator's documentation, we need to resolve other issues
https://issues.apache.org/jira/browse/AIRFLOW-3811 (this issues creates a new subpage for each class)

It is also useful to resolve another issues, but it's not required
#4667 (I invite you to the discussion. Read the description carefully, not just the title. This is a big change for Airflow. I am collecting opinions about this change. Based on the opinion, I will propose a general improvement plan. )

@mik-laj mik-laj deleted the cleaning-integration-rst branch February 11, 2019 02:42
wmorris75 pushed a commit to modmed/incubator-airflow that referenced this pull request Jul 29, 2019
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