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

docs(dart): add link to source code / repository for pub.dev #270

Merged
merged 3 commits into from
May 24, 2023

Conversation

IchordeDionysos
Copy link
Contributor

@IchordeDionysos IchordeDionysos commented May 18, 2023

Related Issue or Design Document

Follow up of #263

Adds the repository to the pubspec.yaml so that on pub.dev a link to the repository will be shown to users.
https://dart.dev/tools/pub/pubspec#repository

Checklist

  • I have read the contributing guidelines and signed the CLA.
  • I have referenced an issue containing the design document if my change introduces a new feature.
  • I have read the security policy.
  • I confirm that this pull request does not address a security vulnerability.
    If this pull request addresses a security vulnerability,
    I confirm that I got approval (please contact security@ory.sh) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation within the code base (if appropriate).

Further comments

I, unfortunately, couldn't test it locally as I get errors for both the current Docker image v0.51.0 as well as a locally built image ...
Not sure if this might have to do something with my M1 MacBook?!

@IchordeDionysos
Copy link
Contributor Author

@jonas-jonas this also allows users to update to dio v5.0.0 fixing two vulnerabilities
https://pub.dev/packages/dio/changelog

https://github.com/advisories?query=type%3Areviewed+ecosystem%3Apub+dio

Copy link
Member

@jonas-jonas jonas-jonas left a comment

Choose a reason for hiding this comment

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

Very nice, this looks good from my POV.

Not sure if this might have to do something with my M1 MacBook?!

The Dart executable segfaults for me too, which is likely due to the amd64 emulation going on. I rented a Linux VM to check the changes.

Should the dio bump to ^5.0.0 be considered a breaking change?

@jonas-jonas jonas-jonas self-assigned this May 22, 2023
@IchordeDionysos
Copy link
Contributor Author

Breaking change or not:

  • Content Type is not longer implicitly set (should be fine as the content type is explicitly set: see)
  • The OpenAPI generator should abstract the other breaking changes or the features are not used
  • The only reason I see to justify a breaking change of Ory would be that Ory now requires using Dio 5.0.0, and the latest version of the Ory SDKs only works with Dio 5.x.

I'm not sure but I guess the Ory SDK would find the latest version of Ory that uses Dio 4.x. Otherwise we'd have package users required to update their packages all to the latest versions which they anyways should do to ensure the best compatibility.

@IchordeDionysos
Copy link
Contributor Author

IchordeDionysos commented May 22, 2023

I rented a Linux VM to check the changes.

I've actually figured out that I can run open-api-generator in the Docker container and then run
dart pub build_runner build outside of the Docker container which works for me

@jonas-jonas
Copy link
Member

I've actually figured out that I can run open-api-generator in the Docker container and then run
dart pub build_runner build outside of the Docker container which works for me

Ah, good idea.

The only reason I see to justify a breaking change of Ory would be that Ory now requires using Dio 5.0.0, and the latest version of the Ory SDKs only works with Dio 5.x.

Is that not the case? The generated version constraint looks like it requires >5.

@IchordeDionysos
Copy link
Contributor Author

IchordeDionysos commented May 24, 2023

Is that not the case? The generated version constraint looks like it requires >5.
Wait, let me explain it better :D

The current Ory SDKs require dio version >= 4.0.0 and < 5.0.0, reducing scores on pub.dev
image

With the change, the Ory SDKs would require dio version >= 5.0.0 and < 6.0.0 (the latest version of dio is 5.1.2)


So the only change that could make it a breaking change would be because the Ory SDKs now depend on dio version 5.x.x.
Not because there are any changes to the Ory SDKs behavior

According to the server FAQ it should probably not be considered a breaking change:
https://semver.org/#what-should-i-do-if-i-update-my-own-dependencies-without-changing-the-public-api
That's maybe also the reason why the maintainers at open-api-generator did not include in the next major version

@aeneasr aeneasr merged commit 3dca508 into ory:master May 24, 2023
@jonas-jonas
Copy link
Member

@IchordeDionysos thank you!

@IchordeDionysos IchordeDionysos deleted the chore/dart-repository branch June 10, 2023 08:01
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