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

Remove reference to Basic Authentication in the docs #2555

Merged
merged 5 commits into from
Aug 30, 2022

Conversation

rkreisel
Copy link
Contributor

Edited the section on Authentication to remove Basic Auth, which GitHub no longer supports, with instructions for using a Personal Access Token.

Edited the section on Authentication to remove Basic Auth, which GitHub no longer supports, with instructions for using a Personal Access Token.
@JonruAlveus
Copy link
Collaborator

Fixes #2554

Copy link
Collaborator

@JonruAlveus JonruAlveus left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! ⭐

@rkreisel
Copy link
Contributor Author

rkreisel commented Aug 26, 2022 via email

Copy link
Collaborator

@JonruAlveus JonruAlveus left a comment

Choose a reason for hiding this comment

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

Sorry just noticed one thing

docs/getting-started.md Outdated Show resolved Hide resolved
@JonruAlveus
Copy link
Collaborator

Hey Chris, I've only ever used Git Hub for my personal projects. This was the first PR I have ever done to a public project. Is there something further I need to do to get this moved to the main branch? Thanks, Randy

---------------------------------------- From: "Chris Simpson" @.> Sent: 8/25/22 11:42 PM To: "octokit/octokit.net" @.> Cc: Randy Kreisel @.>, Author @.> Subject: Re: [octokit/octokit.net] Update getting-started.md (PR #2555) @JonruAlveus approved this pull request. Thanks for the contribution! ⭐ — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

Hi Randy,

Firstly, thanks for the contribution! Around merging the PR, after the one change I think we're good to go. Once that's done I or one of the other Contributers can merge the PR. I believe our primary merger is currently on holiday 😄

Second commit: Removed "/en" from links.
Copy link
Contributor

@timrogers timrogers left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to make this improvement to our docs ❤️ I'd like to make some suggestions to make it even better.

docs/getting-started.md Outdated Show resolved Hide resolved

### Authenticated Access

If you want to access private repositories or perform actions on behalf of a user, you need to pass credentials to the client.

There are two options supported by the API - basic and OAuth authentication.
There are two Authentication options supported by the API.
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't quite true. But that's not your fault, as GitHub's authentication options are very very complicated 🙈 I think I would correct this and just say something like this:

Suggested change
There are two Authentication options supported by the API.
You can authenticate by passing an access token, for example a personal access token (PAT), an OAuth access token or an installation token from a GitHub App

Copy link
Contributor

Choose a reason for hiding this comment

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

(As well as accepting this suggestion, you would need to remove the bullet pointed list below it.)

rkreisel and others added 3 commits August 29, 2022 10:15
Co-authored-by: Tim Rogers <timrogers@github.com>
Co-authored-by: Tim Rogers <timrogers@github.com>
Removed the two bullet pointed items because the sentence preceding them provided a clearer description of the options.
@timrogers timrogers changed the title Update getting-started.md fix: remove reference to Basic Authentication from the docs Aug 30, 2022
@timrogers timrogers changed the title fix: remove reference to Basic Authentication from the docs Remove reference to Basic Authentication in the docs Aug 30, 2022
@timrogers timrogers merged commit 269201c into octokit:main Aug 30, 2022
@nickfloyd
Copy link
Contributor

release_notes: Removed reference to Basic Authentication in the docs

@nickfloyd nickfloyd added Type: Documentation Improvements or additions to documentation and removed category: docs-and-samples labels Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants