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

chore(dynamite_runtime): fix last pana complaints for the 0.1.0 release #1349

Merged
merged 3 commits into from
Dec 26, 2023

Conversation

Leptopoda
Copy link
Member

I'm not sure whether the repository url should really point to the root repo.
both json_serializable and built_value (to name two larger monorepos) point the repo url to the directory the package lives in:
https://github.com/google/json_serializable.dart/blob/master/json_serializable/pubspec.yaml
https://github.com/google/built_value.dart/blob/master/built_value/pubspec.yaml

Attached are the pana results but the only remaining issue is that pana can't find the package in the clone of the root repository 🤷‍♀️
dynamite_pana.txt
dynamite_runtime_pana.txt

@Leptopoda
Copy link
Member Author

Ah and we still can't automatically add the petstore example. For now I just copied the contents to the example directories and adjusted the pubspec_overrides. Long term we should have CI/CD for that.

packages/dynamite/dynamite/CHANGELOG.md Outdated Show resolved Hide resolved
packages/dynamite/dynamite/pubspec.yaml Outdated Show resolved Hide resolved
packages/dynamite/dynamite_runtime/CHANGELOG.md Outdated Show resolved Hide resolved
packages/dynamite/dynamite_runtime/pubspec.yaml Outdated Show resolved Hide resolved
@provokateurin
Copy link
Member

pana complains that web is not supported in dynamite_runtime, but it is because we are using universal_io. Please add this to the pubspec:

platforms:
  windows:
  linux:
  macos:
  android:
  ios:
  web:

For dynamite configuring the platforms doesn't really make sense, but we could limit it to linux, macos and windows

@provokateurin
Copy link
Member

provokateurin commented Dec 23, 2023

Ah and we still can't automatically add the petstore example. For now I just copied the contents to the example directories and adjusted the pubspec_overrides. Long term we should have CI/CD for that.

Even though it's stupid, we could just always copy the package to the example folders when generating it. Then we have 3 copies in the repo.

We could also just make it the example package of dynamite. For dynamite_runtime we don't really need to care about the score. This would be cleaner imo

@Leptopoda
Copy link
Member Author

We could also just make it the example package of dynamite.

I'd rather do that and drop the other packages.
The runtime can just have a README with a link.

@provokateurin
Copy link
Member

I'm not sure whether the repository url should really point to the root repo.

Yeah please point it to the path as well.

@Leptopoda
Copy link
Member Author

Why not just switch it around so the homepage points to the root?
I don't think homepage and repository should point to the same.

@provokateurin
Copy link
Member

Pana will try to check for the package using the repository url. I'm not sure how it is supposed to be used, so please just do what pana wants us to do.

@Leptopoda Leptopoda force-pushed the chore/dynamite/prepare-0.1.0-release branch from 8375675 to 05131a9 Compare December 23, 2023 11:32
@Leptopoda
Copy link
Member Author

Ok cspell needs adjusting but review is appreciated.
I don't think we can make pana happy with the repo url but at least it's now aligned with how built_value and json_serializable do it.

@Leptopoda Leptopoda force-pushed the chore/dynamite/prepare-0.1.0-release branch 2 times, most recently from 66c1879 to a094ca0 Compare December 23, 2023 15:39
@Leptopoda
Copy link
Member Author

any news here?
I think dynamite might be ready :)

@provokateurin
Copy link
Member

please rebase

@provokateurin
Copy link
Member

and run pana again

@Leptopoda Leptopoda force-pushed the chore/dynamite/prepare-0.1.0-release branch from a094ca0 to 69c89c6 Compare December 25, 2023 14:34
@Leptopoda
Copy link
Member Author

Pana still complains about the pubspec and the missing changelog.

packages/dynamite/dynamite/pubspec.yaml Outdated Show resolved Hide resolved
packages/dynamite/dynamite_runtime/pubspec.yaml Outdated Show resolved Hide resolved
melos.yaml Show resolved Hide resolved
@provokateurin
Copy link
Member

Can you rebase and push the issue tracker fix? Then this is ready to be merged

@Leptopoda Leptopoda force-pushed the chore/dynamite/prepare-0.1.0-release branch from 69c89c6 to 5a63947 Compare December 26, 2023 10:11
@Leptopoda Leptopoda enabled auto-merge December 26, 2023 10:13
@Leptopoda Leptopoda merged commit b0cfadb into main Dec 26, 2023
8 checks passed
@Leptopoda Leptopoda deleted the chore/dynamite/prepare-0.1.0-release branch December 26, 2023 10:34
@Leptopoda Leptopoda removed this from the Dynamite packages release milestone May 3, 2024
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.

2 participants