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 createitems #3302

Merged
merged 4 commits into from
Jul 30, 2020
Merged

Remove createitems #3302

merged 4 commits into from
Jul 30, 2020

Conversation

MadeByMike
Copy link
Contributor

Final PR in series, needs to be merged after #3300 and #3129

This is breaking and should probably be shipped after people have had time to make changes based on the above 2 PRs.

@changeset-bot
Copy link

changeset-bot bot commented Jul 28, 2020

🦋 Changeset is good to go

Latest commit: b784881

We got this.

This PR includes changesets to release 18 packages
Name Type
@keystonejs/keystone Major
@keystonejs/adapter-knex Patch
@keystonejs/adapter-mongoose Patch
@keystonejs/test-utils Patch
@keystonejs/demo-project-blog Patch
@keystonejs/demo-custom-fields Patch
@keystonejs/demo-project-meetup Patch
@keystonejs/demo-project-todo Patch
@keystonejs/cypress-project-access-control Patch
@keystonejs/cypress-project-basic Patch
@keystonejs/cypress-project-client-validation Patch
@keystonejs/cypress-project-login Patch
@keystonejs/cypress-project-social-login Patch
@keystonejs/benchmarks Patch
@keystonejs/example-projects-blank Patch
@keystonejs/example-projects-nuxt Patch
@keystonejs/example-projects-starter Patch
@keystonejs/example-projects-todo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@singhArmani singhArmani left a comment

Choose a reason for hiding this comment

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

Along with some PR review suggestions, we fixed some of the tests in the previous branches (#3300 & #3301) of this PR series. Let's hold on till the first PR #3300 gets merged.

@singhArmani singhArmani force-pushed the remove-keysnot-createitems branch 2 times, most recently from d992b92 to 469fe60 Compare July 29, 2020 05:38
@singhArmani singhArmani changed the title WIP: Remove createitems Remove createitems Jul 29, 2020
Copy link
Contributor

@timleslie timleslie left a comment

Choose a reason for hiding this comment

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

There are a couple of files changed in this PR that don't need to.

@@ -2,7 +2,11 @@ const runQuery = async ({ keystone, query, variables, context }) => {
const { data, errors } = await keystone.executeGraphQL({
context:
context ||
keystone.createContext({ schemaName: 'public', authentication: {}, skipAccessControl: true }),
keystone.createContext({
Copy link
Contributor

Choose a reason for hiding this comment

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

This file shouldn't have changed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it happens due to yarn format. 🤔

yarn.lock Outdated
version "2.3.0"
resolved "https://registry.yarnpkg.com/apollo-engine-reporting/-/apollo-engine-reporting-2.3.0.tgz#3bb59f81aaf6b967ed098896a4a60a053b4eed5a"
integrity sha512-SbcPLFuUZcRqDEZ6mSs8uHM9Ftr8yyt2IEu0JA8c3LNBmYXSLM7MHqFe80SVcosYSTBgtMz8mLJO8orhYoSYZw==
apollo-engine-reporting@^2.2.1:
Copy link
Contributor

Choose a reason for hiding this comment

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

This file shouldn't have changed.

@singhArmani singhArmani force-pushed the remove-keysnot-createitems branch 2 times, most recently from c40b816 to 5ee1392 Compare July 29, 2020 06:25
'@keystonejs/keystone': major
---

Removed the `keystone.createItems` method. This has been replaced with the `createItems` function in `@keystonejs/server-side-graphql-client`.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also mention that we removed keystone.createItem.

packages/keystone/README.md Show resolved Hide resolved
packages/keystone/lib/Keystone/index.js Show resolved Hide resolved
@@ -74,284 +74,3 @@ module.exports = {
```

> **Tip:** A similar setup can be achieved by running the Keystone CLI `yarn create keystone-app` and selecting `Starter (Users + Authentication)`. This starter project has a `User` list, `PasswordAuthStrategy` and seeding of the database already configured. For now, we will proceed manually.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing this documentation has rendered this document useless. We should replace the old content with new content which refers to the server-side-graphql-client package. It needn't be as verbose as this content was, since we aren't describing a new APi, but this tutorial should continue to serve a purpose.

@timleslie timleslie mentioned this pull request Jul 30, 2020
@singhArmani singhArmani requested a review from timleslie July 30, 2020 03:13
@singhArmani singhArmani force-pushed the remove-keysnot-createitems branch from 9cfd5a4 to 55d7ef1 Compare July 30, 2020 03:14
Copy link
Contributor

@timleslie timleslie left a comment

Choose a reason for hiding this comment

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

This process is also called seeding. using the createItems method

should be

This process is also called seeding. using the createItems function

A quick run of yarn format and then I think this is ✅

MadeByMike and others added 3 commits July 30, 2020 14:28
- add seeding tutorials
- remove relationship-utils module
- remove leftover `createItem` usage in the docs
@singhArmani singhArmani force-pushed the remove-keysnot-createitems branch from d943e0f to 2ce474b Compare July 30, 2020 04:29
@singhArmani singhArmani force-pushed the remove-keysnot-createitems branch from 2ce474b to b784881 Compare July 30, 2020 04:31
Copy link
Contributor

@timleslie timleslie left a comment

Choose a reason for hiding this comment

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

Looks good! 🚢

@singhArmani singhArmani merged commit 22b4a5c into master Jul 30, 2020
@singhArmani singhArmani deleted the remove-keysnot-createitems branch July 30, 2020 04:50
This was referenced Jul 30, 2020
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.

3 participants