-
Notifications
You must be signed in to change notification settings - Fork 32
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(dynamite)!: validate version of dynamite_runtime when generating #1228
feat(dynamite)!: validate version of dynamite_runtime when generating #1228
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not only check dynamite_runtime but also every package that the generated code depends on (https://github.com/nextcloud/neon/blob/main/packages/dynamite/dynamite/lib/src/builder/imports.dart).
I don't know if this is a good idea. |
I'd say if a package is missing from the pubspec then we can skip it since it might not be used as you said. |
Downgrading for the test should not be needed if we assume devs follow semver (and the packages we choose should come from reputable publishers like we already do). While we (nextcloud package) might be affected by a bug/need a feature does not mean other users face the same. |
I disagree, we should require dependencies that work for everyone, regardless of the features they use. Otherwise it makes no sense to check for dependency versions. |
39fffd8
to
3ed6a9b
Compare
3ed6a9b
to
8d47d09
Compare
packages/dynamite/dynamite/lib/src/helpers/version_checker.dart
Outdated
Show resolved
Hide resolved
packages/dynamite/dynamite/lib/src/helpers/version_checker.dart
Outdated
Show resolved
Hide resolved
packages/dynamite/dynamite/lib/src/helpers/version_checker.dart
Outdated
Show resolved
Hide resolved
packages/dynamite/dynamite/lib/src/helpers/version_checker.dart
Outdated
Show resolved
Hide resolved
8d47d09
to
1482d5f
Compare
forgot to push yesterday 😅 |
packages/dynamite/dynamite/lib/src/helpers/version_checker.dart
Outdated
Show resolved
Hide resolved
packages/dynamite/dynamite/lib/src/helpers/version_checker.dart
Outdated
Show resolved
Hide resolved
packages/dynamite/dynamite/lib/src/helpers/version_checker.dart
Outdated
Show resolved
Hide resolved
packages/dynamite/dynamite/lib/src/helpers/version_checker.dart
Outdated
Show resolved
Hide resolved
packages/dynamite/dynamite/lib/src/helpers/version_checker.dart
Outdated
Show resolved
Hide resolved
1482d5f
to
0288c42
Compare
Please adjust the built_value_generator minimum to 8.8.1 (also in the dynamite docs): #1274. |
0288c42
to
19e7e1c
Compare
packages/dynamite/dynamite/lib/src/helpers/version_checker.dart
Outdated
Show resolved
Hide resolved
19e7e1c
to
8a9f3a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one question
Signed-off-by: Nikolas Rimikis <[email protected]>
Signed-off-by: Nikolas Rimikis <[email protected]>
8a9f3a4
to
40c5f3e
Compare
fixes: #1167