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

Contentful next cleanup #26238

Merged

Conversation

axe312ger
Copy link
Collaborator

@axe312ger axe312ger commented Aug 5, 2020

Implements feedback from @pvdz in #26102

Most notably the removal of console.time() improves the logging further:

Screenshot 2020-08-05 at 13 51 37

@axe312ger axe312ger requested a review from pvdz August 5, 2020 11:02
@axe312ger axe312ger requested a review from a team as a code owner August 5, 2020 11:02
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Aug 5, 2020
const normalize = require(`./normalize`)
const fetchData = require(`./fetch`)
const { createPluginConfig, validateOptions } = require(`./plugin-options`)
const { downloadContentfulAssets } = require(`./download-contentful-assets`)
const { createClient } = require(`contentful`)

Copy link
Contributor

Choose a reason for hiding this comment

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

Keep it but I'm curious whether there's a particular reason for moving the import up. It implies it's relevant for side effects and I'm hoping it's not :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need it for resolving, so we need it always. Dependencies I need in all cases I do require on top. Would you require "on runtime" down there in line 200 when the client is created? 🤔

@axe312ger axe312ger added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Aug 5, 2020
@gatsbybot gatsbybot merged commit 221c70c into gatsbyjs:contentful-next Aug 5, 2020
@axe312ger axe312ger deleted the contentful-next-cleanup branch August 5, 2020 13:37
@pvdz pvdz added topic: source-contentful Related to Gatsby's integration with Contentful and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Aug 11, 2020
axe312ger added a commit to axe312ger/gatsby that referenced this pull request Oct 2, 2020
* improve comment about contentful-resolve-response usage

* test(unit): remove cross test data pollution

* further improve loggin by replacing console.time with GatsbyReporter

* improve code style

* missing file

* remove logging verbosity

* adjust mocked reporter for tests to pass

* remove unused deep-map dependency
axe312ger added a commit to axe312ger/gatsby that referenced this pull request Oct 2, 2020
* improve comment about contentful-resolve-response usage

* test(unit): remove cross test data pollution

* further improve loggin by replacing console.time with GatsbyReporter

* improve code style

* missing file

* remove logging verbosity

* adjust mocked reporter for tests to pass

* remove unused deep-map dependency
pvdz pushed a commit that referenced this pull request Oct 5, 2020
…e performance (#27244)

* fix(gatsby-source-contentful): fixed id collision in contentful entries (#23514)

* fix(gatsby-source-contentful): fixed id collision in contentful entries

* fix(gatsby-source-contentful): properly resolve subsequent sync calls   (#15084)

* fix(gatsby-source-contentful): fixed id collision in contentful entries (#23514)

* fix(gatsby-source-contentful): fixed id collision in contentful entries

* improve logging and clean up code (#26238)

* improve comment about contentful-resolve-response usage

* test(unit): remove cross test data pollution

* further improve loggin by replacing console.time with GatsbyReporter

* improve code style

* missing file

* remove logging verbosity

* adjust mocked reporter for tests to pass

* remove unused deep-map dependency

* fix: use content type id by default to create reference gql types (#26102)

* temporary console.time calls for performance measurements

* perf(gatsby-source-contentful): use a map and less loops to merge old and latest sync API response

* log time Contentful API processing  and GraphQL node creation

* upgrade dependencies and use faster node 10 versions

* remove duplicate logging

* allow multiple Contentful sources

fixes #26875

* refactor(imports): fix dependency structure and make sharp optional

fixes #23904

* align tests to match new multi source cache pattern

Co-authored-by: Matthew Miller <mnmiller1@me.com>
Co-authored-by: Khaled Garbaya <khaled@contentful.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes topic: source-contentful Related to Gatsby's integration with Contentful
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants