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

feat: @nestjs/graphql optionalDependencies #84

Merged
merged 2 commits into from
Aug 12, 2020

Conversation

9renpoto
Copy link
Collaborator

closes #58

optional @nestjs/graphql modules

@9renpoto 9renpoto self-assigned this Jul 25, 2020
@9renpoto 9renpoto added the enhancement New feature or request label Jul 25, 2020
@github-actions
Copy link
Contributor

Pull Request Test Coverage Report for Build 182115833

  • 3 of 4 (75.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.5%) to 85.556%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/raven.interceptor.ts 3 4 75.0%
Totals Coverage Status
Change from base Build 179158934: -1.5%
Covered Lines: 48
Relevant Lines: 53

💛 - Coveralls

@9renpoto 9renpoto marked this pull request as ready for review July 25, 2020 08:11
@9renpoto 9renpoto requested a review from mentos1386 July 25, 2020 08:11
Copy link
Owner

@mentos1386 mentos1386 left a comment

Choose a reason for hiding this comment

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

Awesome!
It would be nice if we would have tests to cover this. But I'm guessing it would be hard to write them?

@9renpoto 9renpoto added this to the v7.0.0 milestone Aug 6, 2020
Comment on lines +55 to +56
if (!GqlArgumentsHost)
return this.captureException(scope, exception);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
if (!GqlArgumentsHost)
return this.captureException(scope, exception);

If you don't use @nestjs/graphql, we don't come under this condition. 🤔 💭

@@ -9,7 +9,7 @@
"build": "rm -rf dist && tsc -p tsconfig.build.json",
"format": "prettier \"{example,lib,test}/**/*.ts\" --write",
"lint": "prettier \"{example,lib,test}/**/*.ts\" --check",
"prepublish": "npm run build",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

npm WARN prepublish-on-install As of npm@5, `prepublish` scripts are deprecated.
npm WARN prepublish-on-install Use `prepare` for build steps and `prepublishOnly` for upload-only.
npm WARN prepublish-on-install See the deprecation note in `npm help scripts` for more information.

@9renpoto 9renpoto merged commit 1bb1125 into master Aug 12, 2020
@9renpoto 9renpoto mentioned this pull request Aug 19, 2020
@9renpoto 9renpoto deleted the feature/optional-graphql branch August 19, 2020 09:10
@9renpoto 9renpoto mentioned this pull request Aug 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot find module '@nestjs/graphql'
2 participants