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

Android guidelines restructuring #2324

Merged
merged 9 commits into from
Feb 18, 2021

Conversation

vcolin7
Copy link
Member

@vcolin7 vcolin7 commented Feb 3, 2021

Fixes issue #482 from the Android repository.

Re-organized the Android guidelines following the @bsiegel's effort for iOS. There are a number of TODO items, but I think it's important to get the ball rolling with a PR to get valuable feedback. These guidelines took heavy inspiration from the iOS guidelines where appropriate, so much of the feedback given by @annelo-msft will be reflected here as well.

Copy link
Member

@annelo-msft annelo-msft left a comment

Choose a reason for hiding this comment

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

Approved, pending resolution of comments.

@@ -41,47 +48,35 @@ The HTTP pipeline consists of a HTTP transport that is wrapped by multiple polic

### Supporting Types

Copy link
Member

Choose a reason for hiding this comment

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

nit: you took out the TODO without adding the into sentence 😊 (also a couple places below)

Copy link
Member Author

Choose a reason for hiding this comment

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

I noticed the Java guidelines do not have an introductory sentence for these sections and assumed it was not required. Do we still want to add one?

@annelo-msft
Copy link
Member

nit:

On the top level nav bar, there are still pages that will need to be consolidated:

image

This is what it should look like:

image

@vcolin7
Copy link
Member Author

vcolin7 commented Feb 17, 2021

nit:

On the top level nav bar, there are still pages that will need to be consolidated:

image

This is what it should look like:

image

I thought I had already made this change, thanks for pointing it out :)

@vcolin7
Copy link
Member Author

vcolin7 commented Feb 18, 2021

@annelo-msft I did remove the additional links from the general_sidebar.yml file, so I'm not sure why you can see them when generating the HTML. Is there another place I should remove them from?

@annelo-msft
Copy link
Member

I was looking at the published site as part of my status check - if you've generated the HTML and they're gone, then we're all good. Apologies for the false alarm.

@vcolin7 vcolin7 requested a review from srnagar as a code owner February 18, 2021 21:16
@vcolin7 vcolin7 merged commit a091066 into Azure:master Feb 18, 2021
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.

2 participants