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: fix the rust build #27

Merged
merged 1 commit into from
Mar 6, 2023
Merged

chore: fix the rust build #27

merged 1 commit into from
Mar 6, 2023

Conversation

eseidel
Copy link
Contributor

@eseidel eseidel commented Mar 6, 2023

This brings over all of the updater rust code from old_repo, except I deleted the "publisher" package which is now covered in the shorebird cli and I used the updated "shorebird_code_push_updater" package as "library".

I restored building with the rust and cdylib variants and kept the UPDATER_DEMO docs (we'll delete them soon but they still have some build instructions). I edited down the README a bit since some of the instructions were old.

Once this is checked in, I'll work with felix to organize the code more aligned with his vision for the repo layout.

This brings over all of the updater rust code from old_repo, except I deleted the "publisher" package which is now covered in the shorebird cli and I used the updated "shorebird_code_push_updater" package as "library".

I restored building with the rust and cdylib variants and kept the UPDATER_DEMO docs (we'll delete them soon but they still have some build instructions).  I edited down the README a bit since some of the instructions were old.

Once this is checked in, I'll work with felix to organize the code more aligned with his vision for the repo layout.
@@ -0,0 +1,381 @@
# Generated by pub
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on adding the pubspec.lock to the .gitignore since I believe the lock file is not taken into consideration when the CLI is activated via pub.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suspect it doesn't matter here much. Technically this should commit today, but won't once we move it to be a library? https://dart.dev/guides/libraries/private-files

Copy link
Contributor

Choose a reason for hiding this comment

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

I think Dart CLIs are treated like libraries.

For library packages, don’t commit the pubspec.lock file. Regenerating the pubspec.lock file lets you test your package against the latest compatible versions of its dependencies.

Other CLIs like pub don't commit the lock file either https://github.com/dart-lang/pub but yeah it's not a big deal either way.

@@ -0,0 +1,128 @@
To replicate the updater demo, you'll need a copy of the Flutter Engine.
Copy link
Contributor

Choose a reason for hiding this comment

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

Most of this should hopefully be able to go away very soon :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct.

Copy link
Contributor

@felangel felangel left a comment

Choose a reason for hiding this comment

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

Overall LGTM

@felangel felangel changed the title chore: Fix the rust build chore: fix the rust build Mar 6, 2023
@eseidel eseidel merged commit c13091b into main Mar 6, 2023
@eseidel eseidel deleted the fix_rust_build branch March 6, 2023 23:06
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