Skip to content

Conversation

@wolfy1339
Copy link
Member

@wolfy1339 wolfy1339 commented Jan 3, 2023

Followup to octokit/endpoint.js#382 (comment)
See also octokit/endpoint.js#388 octokit/core.js#538 octokit/plugin-paginate-rest.js#486


Behavior

Before the change?

  • Previews were documented

After the change?

  • Previews are no longer documented

Other information

  • This PR must be merged before all other ones in the Octokit JS repos

Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • Removal of previews

Pull request type

Please add the corresponding label for change this PR introduces:

  • Bugfix: Type: Bug
  • Feature/model/API additions: Type: Feature
  • Updates to docs or samples: Type: Documentation
  • Dependencies/code cleanup: Type: Maintenance

@wolfy1339 wolfy1339 added Type: Breaking change Used to note any change that requires a major version bump Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR labels Jan 3, 2023
@nickfloyd nickfloyd merged commit cb0835b into main Jan 11, 2023
@nickfloyd nickfloyd deleted the remove-previews branch January 11, 2023 17:59
@octokitbot
Copy link
Contributor

🎉 This PR is included in version 8.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@wolfy1339
Copy link
Member Author

This unfortunately removes all previews. A follow up is needed to add it from GraphQL

@kfcampbell
Copy link
Contributor

@wolfy1339 do you think it's worth a revert of this PR in the meantime? I'm not sure I understand the scope of the issue.

@wolfy1339
Copy link
Member Author

I don't think a revert is warranted here.

We can probably handle it in the packages that have GraphQL themselves.

  • @octokit/core
  • @octokit/graphql
  • octokit (octokit.js)

Types would need to be handled there

@nickfloyd
Copy link
Contributor

It looks like this broke plugin-enterprise-server.js as well - I should've taken a closer look and remembered that there are still old supported versions of GHE that have references to previews.

@nickfloyd
Copy link
Contributor

On the upside, GHE 3.2 (which had the majority of the preview implementation is deprecated: https://docs.github.com/en/enterprise-server@3.7/admin/all-releases

I'll need to check 3.3 (set to deprecate next week 01.18.2023) and 3.4 to make sure they are all gone from GHE versions - if that's the case we can just drop support for those versions as well and not rollback.

Thoughts?

@nickfloyd
Copy link
Contributor

Update: Two instances have been left out in the REST API descriptions though they are defunct by v3.3.

@wolfy1339 It looks like we have two options here:

  1. Revert
  2. Clean up the files like this and deprecate v3.2 and v3.3 and move on.

Let me know your thoughts when you get the chance. Thanks!

@wolfy1339
Copy link
Member Author

Sounds good

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

Labels

released Type: Breaking change Used to note any change that requires a major version bump Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants