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

Upgrade web_socket_channel to ^3.0.1 #1460

Open
Chralu opened this issue Oct 3, 2024 · 18 comments
Open

Upgrade web_socket_channel to ^3.0.1 #1460

Chralu opened this issue Oct 3, 2024 · 18 comments
Labels
needs more info More information is required Priority: Low Low priority, would be cool to include inside the next release PRs encouraged PRs are welcome if stakeholders want to take ownership

Comments

@Chralu
Copy link

Chralu commented Oct 3, 2024

No description provided.

Copy link

request-info bot commented Oct 3, 2024

We would appreciate it if you could provide us with more info about this issue/pr!

@request-info request-info bot added the needs more info More information is required label Oct 3, 2024
@vincenzopalazzo vincenzopalazzo added PRs encouraged PRs are welcome if stakeholders want to take ownership Priority: Low Low priority, would be cool to include inside the next release labels Oct 3, 2024
@redDwarf03
Copy link

hello @Chralu,
In the meantime, you can use this branch https://github.com/redDwarf03/graphql-flutter/tree/Web_socket_channel_301

Note that I've removed the examples for null safety reasons. I didn't see the point of updating in view of the new version that will be released.

@vincenzopalazzo
Copy link
Collaborator

Note that I've removed the examples for null safety reasons. I didn't see the point of updating in view of the new version that will be released.

@redDwarf03 probably because your branch is not in sync with main, I see a lot of conflict inside the PR that you open

@redDwarf03
Copy link

Note that I've removed the examples for null safety reasons. I didn't see the point of updating in view of the new version that will be released.

@redDwarf03 probably because your branch is not in sync with main, I see a lot of conflict inside the PR that you open

I forked 5.1.2 and not 5.2.0 because 5.2.0 is a beta version not available in stable version

@vincenzopalazzo
Copy link
Collaborator

I forked 5.1.2 and not 5.2.0 because 5.2.0 is a beta version not available in stable version

What is wrong with the "beta" version? in reality, you should use the 5.2 because the beta is just a staging version

@hagen00
Copy link
Contributor

hagen00 commented Oct 18, 2024

A number of packages now require web_socket_channel greater than version 3. web_socket_channel version 3.0.0 was released 5 months ago.

Please update the beta version of graphql-flutter to make use of web_socket_channel 3.0.0 and up! Latest version of web_socket_channel is 3.0.1

@vincenzopalazzo
Copy link
Collaborator

Please update the beta version of graphql-flutter to make use of web_socket_channel 3.0.0 and up! Latest version of web_socket_channel is 3.0.1

Currently not have enough time (outside of my free time) to work on this, if you can and want I will be available to review a PR for it.

@hagen00
Copy link
Contributor

hagen00 commented Oct 20, 2024

PR is here: #1463

graphql_flutter/pubspec.yaml references graphql: ^5.2.0-beta.8 in the dependencies. This will also have to change on merge.

I have tested with my code base and everything seems to work normally. Unfortunately, flutter test doesn't work for me locally. Not sure if it should? Running on Ubuntu.

I do not use graphql-flutter for subscriptions, so that is untested.

@vincenzopalazzo
Copy link
Collaborator

I have tested with my code base and everything seems to work normally. Unfortunately, flutter test doesn't work for me locally. Not sure if it should? Running on Ubuntu.

not sure I usually ran make check.

I do not use graphql-flutter for subscriptions, so that is untested.

Running the CI now, lets see what happens, if it will pass, we will merge it and if there is some problem we will revert the changes

@hagen00
Copy link
Contributor

hagen00 commented Oct 22, 2024

PR has been updated here: #1463

With some extra info.

@mingjunsiek
Copy link

Any update on this PR? Still waiting for this package to use web_socket_channel 3.0.0 and up

@hagen00
Copy link
Contributor

hagen00 commented Nov 5, 2024

Just made a change to the PR. Not sure if further changes to the versioning are required. Hopefully we can merge soon. I've been using my branch like below in production, but would obviously prefer to use the official package.

graphql:
    git:
      url: https://github.com/hagen00/graphql-flutter.git
      ref: main
      path: packages/graphql
  graphql_flutter:
    git:
      url: https://github.com/hagen00/graphql-flutter.git
      ref: main
      path: packages/graphql_flutter

Chralu added a commit to archethic-foundation/archethic-dapp-framework-flutter that referenced this issue Nov 7, 2024
Waiting for zino-hofmann/graphql-flutter#1460 resolution to use stable versions.
Chralu added a commit to archethic-foundation/archethic-dapp-framework-flutter that referenced this issue Nov 7, 2024
Waiting for zino-hofmann/graphql-flutter#1460 resolution to use stable versions.
@redDwarf03
Copy link

@hagen00 any idea about the merge ? thx

@hagen00
Copy link
Contributor

hagen00 commented Dec 10, 2024

Unfortunately I don't know about merging it. @vincenzopalazzo will need to advise. I'm happy to do more work if needed. e.g to cleaning up code formatting etc...I'm also waiting to have it merged :(

You can use my fork like this

graphql:
    git:
      url: https://github.com/hagen00/graphql-flutter.git
      ref: main
      path: packages/graphql
  graphql_flutter:
    git:
      url: https://github.com/hagen00/graphql-flutter.git
      ref: main
      path: packages/graphql_flutter

@redDwarf03
Copy link

Unfortunately I don't know about merging it. @vincenzopalazzo will need to advise. I'm happy to do more work if needed. e.g to cleaning up code formatting etc...I'm also waiting to have it merged :(

You can use my fork like this

graphql:
    git:
      url: https://github.com/hagen00/graphql-flutter.git
      ref: main
      path: packages/graphql
  graphql_flutter:
    git:
      url: https://github.com/hagen00/graphql-flutter.git
      ref: main
      path: packages/graphql_flutter

Thank you but not possible to publish a lib using graphql_flutter with a github dependency 😢

@vincenzopalazzo
Copy link
Collaborator

Sorry for delay, I commented on the PR, when we get that solved we merge it

@hagen00
Copy link
Contributor

hagen00 commented Dec 10, 2024

Thank you @vincenzopalazzo. melos run client_analyze now succeeds and the commit history is as before. Hopefully it's ready for merge

@redDwarf03
Copy link

Sorry for delay, I commented on the PR, when we get that solved we merge it

@vincenzopalazzo Are you ok with last updates on PR? thx

redDwarf03 pushed a commit to archethic-foundation/archethic-dapp-framework-flutter that referenced this issue Dec 13, 2024
Waiting for zino-hofmann/graphql-flutter#1460 resolution to use stable versions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more info More information is required Priority: Low Low priority, would be cool to include inside the next release PRs encouraged PRs are welcome if stakeholders want to take ownership
Projects
None yet
Development

No branches or pull requests

7 participants
@Chralu @mingjunsiek @hagen00 @vincenzopalazzo @redDwarf03 and others