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.

Sign up for GitHub

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

Jump to bottom

Organize Amazon provider docs index #30541

Merged
merged 1 commit into from
Apr 14, 2023
Merged

Organize Amazon provider docs index #30541

merged 1 commit into from
Apr 14, 2023

Conversation

eladkal
Copy link
Contributor

@eladkal eladkal commented Apr 8, 2023 •
edited
Loading

Changes involved:

  1. Need to come up with convention for prefix Amazon / AWS / Amazon AWS ?
  2. Moved Transfers to it's own dedicated page
  3. Should the docs be by service? For example: convert EMR to an index that contain links to the 3 EMR pages (EMR, EMR on EKS and EMR serverless)
  4. List in transfers is not easy to sort

I couldn't find a way to present the list by titles.. we can only present in the order of the files names.

cc @o-nikolas @vincbeck @ferruzzi @shubham22 suggestions will be appreciated!

TODO:

  • add all transfer changes to redirect

Before:
Screenshot 2023-04-08 at 15 02 32

After (not final!):

Screenshot 2023-04-08 at 14 58 58

Screenshot 2023-04-08 at 15 03 13

@eladkal eladkal requested a review from o-nikolas as a code owner April 8, 2023 12:06
@eladkal eladkal changed the title Organize Amazon providers docs index Organize Amazon provider docs index Apr 8, 2023
@uranusjr
Copy link
Member

uranusjr commented Apr 8, 2023

The A in AWS already stands for Amazon, “Amazon AWS” simply does not make sense.

@eladkal
Copy link
Contributor Author

eladkal commented Apr 8, 2023

The A in AWS already stands for Amazon, “Amazon AWS” simply does not make sense.

I agree but this is the current title so I mentioned it...

@eladkal eladkal force-pushed the aws branch 2 times, most recently from fac84cb to 62491ad Compare April 8, 2023 16:02
@eladkal eladkal requested review from potiuk and ashb as code owners April 8, 2023 16:02
@vincbeck
Copy link
Contributor

The A in AWS already stands for Amazon, “Amazon AWS” simply does not make sense.

I agree but this is the current title so I mentioned it...

I personally think we should still drop the Amazon. As @uranusjr mentioned, it does not really make sense to me

vincbeck
vincbeck reviewed Apr 11, 2023
View reviewed changes
Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

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

Overall:

  • I really like this change, to me moving all transfer docs to its own package makes sense and make it easier to read (as a user and as a dev)
  • I like the idea of creating index file for services which have multiple docs (EMR, Redshift, ...)
  • I'd vote for AWS as prefix

@ferruzzi
Copy link
Contributor

ferruzzi commented Apr 11, 2023 •
edited
Loading

I love a good housekeeping PR ❤️

I'll weigh in on the A thing, FWIW IMHO IANAL and any other caveats that may be applicable: When in doubt, I use this as the Source of Truth for official service names (some are Amazon {Foo} while others are AWS {Bar} and there doesn't seem to be any pattern to the decisions) and they refer to is as AWS.

@ferruzzi
Copy link
Contributor

MWAA is not an official name :)

Nope, it's not. Just the unofficial abbreviation because who wants to say/type that whole thing out every time :P

@o-nikolas
Copy link
Contributor

Need to come up with convention for prefix Amazon / AWS / Amazon AWS ?

As I mentioned in one of the comments above, I think we should call it what it is. Some services are officially AWS {Foo} (like AWS Batch) while others are officially Amazon {Bar} (like Amazon SageMaker). The AWS Docs page is IMHO the official Source of Truth. So we should match those names. For example, you've changes Athena to AWS Athena in some places but not in others, those should be changed back to Amazon Athena per their official doc page

I have to agree with Dennis on this one I think. The AWS vs Amazon prefix for each individual service is a very intentional decision and it is used everywhere across our docs, blogs, etc. It really is considered part of the service name to a strong degree here at Amazon (and some services have neither AWS or Amazon as a prefix!).
I agree it can be a little frustrating Elad, but I think we need to stick with the proper names so that there is brand recognition for our users.

@eladkal
Copy link
Contributor Author

eladkal commented Apr 13, 2023

but I think we need to stick with the proper names so that there is brand recognition for our users.

NP. will modify accordingly

@eladkal eladkal marked this pull request as ready for review April 13, 2023 18:45
@eladkal eladkal requested review from ferruzzi and vincbeck April 13, 2023 18:45
@ferruzzi ferruzzi self-assigned this Apr 13, 2023
@ferruzzi
Copy link
Contributor

ferruzzi commented Apr 13, 2023 •
edited
Loading

Sorry, I accidentally clicked... something... that wasn't intentional. I think I've fixed it.

@ferruzzi ferruzzi assigned eladkal and unassigned ferruzzi Apr 13, 2023
@eladkal eladkal removed their assignment Apr 13, 2023
@eladkal eladkal requested a review from potiuk April 13, 2023 20:16
o-nikolas
o-nikolas reviewed Apr 13, 2023
View reviewed changes
Copy link
Contributor

@o-nikolas o-nikolas left a comment

Choose a reason for hiding this comment

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

Some AWS --> Amazon name changes

potiuk
potiuk reviewed Apr 14, 2023
View reviewed changes
Copy link
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.

Looks like one more `../' missing but other than that - LGTM. Glad that you sorted out AWS vs. Amazon vs. ... namings :)

@eladkal
Copy link
Contributor Author

eladkal commented Apr 14, 2023 •
edited
Loading

Looks like one more `../' missing but other than that - LGTM.

Yes due to #28338 that added new rst

fix is coming

@potiuk potiuk merged commit 38f2b4a into apache:main Apr 14, 2023
@eladkal eladkal deleted the aws branch April 14, 2023 18:18
wookiist pushed a commit to wookiist/airflow that referenced this pull request Apr 19, 2023
@eladkal
Organize Amazon providers docs index (apache#30541)
4244c6a
This was referenced Apr 21, 2023
Closed
Closed
@eladkal eladkal mentioned this pull request Apr 26, 2023
Closed
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@o-nikolas o-nikolas o-nikolas left review comments

@potiuk potiuk potiuk approved these changes

@vincbeck vincbeck vincbeck approved these changes

@ferruzzi ferruzzi Awaiting requested review from ferruzzi

Assignees
No one assigned
Labels
area:providers kind:documentation provider:amazon AWS/Amazon - related issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@eladkal @uranusjr @vincbeck @ferruzzi @o-nikolas @potiuk