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

Spelling #167

Merged
merged 7 commits into from Mar 4, 2024
Merged

Spelling #167

merged 7 commits into from Mar 4, 2024

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Feb 18, 2024

Before the change?

After the change?

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)

Does this introduce a breaking change?

-const { paginateGraphql } = require("@octokit/plugin-paginate-graphql");
+const { paginateGraphQL } = require("@octokit/plugin-paginate-graphql");

Please see our docs on breaking changes to help!

  • Yes
  • No

I'm happy to drop that one change. There were enough typos in the neighborhood that I'd like to make the argument that it's better to properly spell GraphQL than to inconsistently spell it including with typos.

@jsoref
spelling: backward
499765c 
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@jsoref
spelling: continually
70ea973 
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@jsoref
spelling: conventions
7a8a5d3 
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@jsoref
spelling: cursor
acff05e 
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@jsoref
spelling: defined
e88a4a5 
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@jsoref
spelling: graphql
961d88c 
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@jsoref
spelling: typescript
735d250 
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
@github-actions GitHub Actions
Copy link

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

jsoref
jsoref commented Feb 18, 2024
View reviewed changes
test/paginate.test.ts
issues(first: 10, after: $curosr) {
issues(first: 10, after: $cursor) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this test was testing what it thought it was testing.

test/typescript-validate.ts
export function pageInfoBackwaredExported(): PageInfoBackward {
export function pageInfoBackwardExported(): PageInfoBackward {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a test, but, um... I'm not sure what it was doing either.

@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 Feb 18, 2024
kfcampbell
kfcampbell approved these changes Mar 4, 2024
View reviewed changes
Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

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

I'm 👍 on this PR, the breaking change is minimal and easy to adjust for and the other changes are helpful. I'll hold off on merging for now as I know @wolfy1339 is doing a lot of (excellent) work releasing in our JS ecosystem right now and I don't want to step on toes. If Wolfy is good with a release here, I can merge/release.

@wolfy1339 wolfy1339 changed the base branch from main to beta March 4, 2024 18:36
@wolfy1339
Copy link
Member

I'm going to put this into the beta branch and just lump it in with my other changes

@wolfy1339 wolfy1339 merged commit c6cc5fd into octokit:beta Mar 4, 2024
8 checks passed
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 4, 2024

🎉 This PR is included in version 5.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jsoref jsoref deleted the spelling branch March 4, 2024 18:59
wolfy1339 added a commit that referenced this pull request Mar 4, 2024
@wolfy1339 @jsoref
feat: v5 (#175)
4d6ff45 
* maint: spelling (#167)

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>

BREAKING CHANGE: change spelling of `paginateGraphql` export to `paginateGraphQL`

* feat: package is now ESM

BREAKING CHANGE: package is now ESM

* fix: add return type annotation

---------

Co-authored-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
kachick added a commit to kachick/wait-other-jobs that referenced this pull request Mar 5, 2024
@kachick
Follow renamed feature
f64343a 
octokit/plugin-paginate-graphql.js#167
@wolfy1339 wolfy1339 mentioned this pull request Mar 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kfcampbell kfcampbell kfcampbell approved these changes

Assignees
No one assigned
Labels
released on @beta 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
🧰 Octokit Active
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jsoref @wolfy1339 @kfcampbell