Skip to content

[AIRFLOW-XXX] Alphabetical table and remove duplicate#6487

Merged
mik-laj merged 1 commit into
apache:masterfrom
zhongjiajie:doc_alphabetical_remove_duplicate
Nov 14, 2019
Merged

[AIRFLOW-XXX] Alphabetical table and remove duplicate#6487
mik-laj merged 1 commit into
apache:masterfrom
zhongjiajie:doc_alphabetical_remove_duplicate

Conversation

@zhongjiajie
Copy link
Copy Markdown
Member

@zhongjiajie zhongjiajie commented Nov 1, 2019

  • Alphabetical Service name in Software operators
    and hooks sections
  • Alphabetical source then destination in transfer
    section
  • Remove duplicate hive_to_mysql

Make sure you have checked all steps below.

Description

  • Alphabetical Service name in Software operators
    and hooks sections
  • Alphabetical source then destination in transfer
    section
  • Remove duplicate hive_to_mysql

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
    • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

@zhongjiajie
Copy link
Copy Markdown
Member Author

zhongjiajie commented Nov 1, 2019

@mik-laj PTAL

mik-laj
mik-laj reviewed Nov 1, 2019
View reviewed changes
Comment thread docs/operators-and-hooks-ref.rst Outdated
Copy link
Copy Markdown
Member

@mik-laj mik-laj Nov 1, 2019

Choose a reason for hiding this comment

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

https://github.com/apache/airflow/pull/6487/files#diff-471d3adab9e55d525a899f46345f3b2fL501
It seems to me that order was introduced here.

Copy link
Copy Markdown
Member Author

@zhongjiajie zhongjiajie Nov 1, 2019

Choose a reason for hiding this comment

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

Sorry, I am afraid I don't quite understand what you mean.

Copy link
Copy Markdown
Member

@mik-laj mik-laj Nov 1, 2019

Choose a reason for hiding this comment

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

We use alphabetical sorting here, but ignoring the "cloud" prefix.
Screenshot 2019-11-01 at 17 26 30

Copy link
Copy Markdown
Member Author

@zhongjiajie zhongjiajie Nov 4, 2019

Choose a reason for hiding this comment

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

Are we sort by hook or operator name rather than 'display' name?

Copy link
Copy Markdown
Member

@mik-laj mik-laj Nov 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

By display name, but we ignore the prefix "Cloud" . This prefix has no added value because its use by Google is not uniform and not ordered.
For example:
When we have services: Compute Engine, Cloud SQL, Cloud Storage, PubSub.
The classic alphabetical order creates the following list.

  • Cloud SQL
  • Cloud Storage
  • Compute Engine
  • PubSub
  • Spanner

However, we want to ignore the Cloud prefix, so the list will look like this:

  • Compute Engine
  • PubSub
  • Spanner
  • SQL=> Cloud SQL
  • Storage => Cloud Storage

This will allow you to easily find services, because some services are not placed at the very end of the list when Google added a prefix, which is not consistently used even by Google.

Copy link
Copy Markdown
Member

@mik-laj mik-laj Nov 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

No. We can accept changes in the content, but I'd rather limit the changes in the Sphinx template. Your PR doesn't change the.template.

Copy link
Copy Markdown
Member Author

@zhongjiajie zhongjiajie Nov 4, 2019

Choose a reason for hiding this comment

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

OK, I will close the PR

Copy link
Copy Markdown
Member

@mik-laj mik-laj Nov 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Please no. I want accept this PR. 😿

Copy link
Copy Markdown
Member Author

@zhongjiajie zhongjiajie Nov 4, 2019

Choose a reason for hiding this comment

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

I think you want to change sphinx template to sorted the content? Am I wrong?

Copy link
Copy Markdown
Member

@mik-laj mik-laj Nov 12, 2019

Choose a reason for hiding this comment

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

The script will provide an additional option, but by default the tables should also be sorted

@zhongjiajie zhongjiajie closed this Nov 4, 2019
@zhongjiajie zhongjiajie reopened this Nov 4, 2019
@zhongjiajie zhongjiajie force-pushed the doc_alphabetical_remove_duplicate branch 3 times, most recently from cc852bf to 0522193 Compare November 7, 2019 05:55
@zhongjiajie
Copy link
Copy Markdown
Member Author

zhongjiajie commented Nov 7, 2019

@mik-laj I change Google service part and ignore the prefix "Cloud", PTAL

@mik-laj
Copy link
Copy Markdown
Member

mik-laj commented Nov 12, 2019

Sorry. I was busy lately because I was working on a new website. Can you do a rebase?

@zhongjiajie zhongjiajie force-pushed the doc_alphabetical_remove_duplicate branch from 8df51fa to a61702b Compare November 12, 2019 06:04
@zhongjiajie
Copy link
Copy Markdown
Member Author

zhongjiajie commented Nov 12, 2019

@mik-laj I rebase on master, PTAL, if you have time. I think the CI will pass cause this PR only doc change.

@zhongjiajie
[AIRFLOW-XXX] Alphabetical table and remove duplicate
dd7331e 
* Alphabetical Service name in Software operators
  and hooks sections
* Alphabetical source then destination in transfer
  section
* Remove duplicate hive_to_mysql
@zhongjiajie zhongjiajie force-pushed the doc_alphabetical_remove_duplicate branch from a61702b to dd7331e Compare November 12, 2019 11:20
@zhongjiajie
Copy link
Copy Markdown
Member Author

zhongjiajie commented Nov 12, 2019

Fix the conflict again.

@mik-laj mik-laj merged commit 4c91980 into apache:master Nov 14, 2019
@zhongjiajie zhongjiajie deleted the doc_alphabetical_remove_duplicate branch November 14, 2019 02:00
@zhongjiajie
Copy link
Copy Markdown
Member Author

zhongjiajie commented Nov 14, 2019

@mik-laj Thanks.

@mik-laj
Copy link
Copy Markdown
Member

mik-laj commented Nov 14, 2019

I thank you.

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

Reviewers

@mik-laj mik-laj mik-laj left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zhongjiajie @mik-laj