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(graphql_common): add common package that contains utils functions #1207

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vincenzopalazzo
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo commented Aug 22, 2022

We have a big lack in the library, we can not trace in case of a fancy problem, with this PR I'm proposing to put a generic interface + a Logger trace inside a separate package, and use it inside the client.

This will help us to safely make refactoring and trace the example that I proposed in other open PRs today!

  • Include the release process
  • make a sanity check of the Makefile

@vincenzopalazzo vincenzopalazzo added the dart client Issue relates mainly to the standalone dart client label Aug 22, 2022
@vincenzopalazzo vincenzopalazzo marked this pull request as ready for review November 27, 2022 20:42
@vincenzopalazzo
Copy link
Collaborator Author

vincenzopalazzo commented Nov 27, 2022

I think this is ready to get merged, but maybe after the official 5.1.2 release and we get it inside the 5.2.0-beta.1

What do you think? @budde377

@vincenzopalazzo
Copy link
Collaborator Author

The CI failure is unrelated we should publish our first common version but let do the more important stuff before

warning - pubspec.yaml:26:5 - Publishable packages can't have 'path' dependencies. Try adding a 'publish_to: none' entry to mark the package as not for publishing or remove the path dependency. - invalid_dependency

@vincenzopalazzo
Copy link
Collaborator Author

@budde377 this should be ready to be merged, and later I need to document it, but for now it is just a internal package so imho it is ready to go

@@ -4,6 +4,9 @@ version: 5.1.3
repository: https://github.com/zino-app/graphql-flutter/tree/main/packages/graphql
issue_tracker: https://github.com/zino-hofmann/graphql-flutter/issues

# just for dev work
publish_to: 'none'

Copy link

Choose a reason for hiding this comment

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

You probably don't want to commit this.

@@ -22,6 +25,8 @@ dependencies:
stream_channel: ^2.1.0
rxdart: ^0.27.1
uuid: ^3.0.1
graphql_common:
path: ../graphql_common
Copy link

Choose a reason for hiding this comment

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

Same here, I guess this is a leftover from development. Since you are using melos a pubspec_overrides.yaml should do the trick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dart client Issue relates mainly to the standalone dart client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants