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.

Sign up for GitHub

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

Jump to bottom

[native_assets_cli] BuildConfig.packageName #146

Merged
merged 1 commit into from
Oct 3, 2023
Merged

[native_assets_cli] BuildConfig.packageName #146

merged 1 commit into from
Oct 3, 2023

Conversation

dcharkes
Copy link
Collaborator

@dcharkes dcharkes commented Oct 2, 2023

Bug: #142

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Package publishing

Package Version Status Publish tag (post-merge)
package:native_assets_cli 0.3.0 ready to publish native_assets_cli-v0.3.0
package:native_assets_builder 0.2.3 already published at pub.dev
package:native_toolchain_c 0.2.5 already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

@coveralls
Copy link

Coverage Status

coverage: 98.178% (+0.006%) from 98.172% when pulling 82d6c4e on package-name into 22500ea on main.

@j4qfrost
Copy link

j4qfrost commented Oct 2, 2023 •
edited
Loading

@dcharkes native_assets_builder needs this extension deleted as well. I don't think it breaks anything, but the issue remains if the extension overrides BuildConfig's packageName.

native/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart

Line 473 in 82d6c4e

extension on BuildConfig {

Thanks for the quick turnaround.

Never mind, I realized this needs to be merged before changing the runner.

@dcharkes
Copy link
Collaborator Author

dcharkes commented Oct 3, 2023

Never mind, I realized this needs to be merged before changing the runner.

Correct. 👍 Thanks for having the extra pair of eyes! 👁️

HosseinYousefi
HosseinYousefi approved these changes Oct 3, 2023
View reviewed changes
Copy link
Member

@HosseinYousefi HosseinYousefi left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @dcharkes!

@dcharkes dcharkes merged commit ec0a7c9 into main Oct 3, 2023
17 checks passed
@dcharkes dcharkes deleted the package-name branch October 3, 2023 12:09
HosseinYousefi added a commit that referenced this pull request Nov 16, 2023
@HosseinYousefi
[jnigen] Updated docs to reflect renames (#146)
534e92f 
* Updated docs to reflect renames (`JniObject` -> `JObject`)
* Updated llvm versions for linux dylib lookup as tests weren't passing
HosseinYousefi added a commit that referenced this pull request Nov 16, 2023
@HosseinYousefi
[jnigen] Updated docs to reflect renames (#146)
cea758e 
* Updated docs to reflect renames (`JniObject` -> `JObject`)
* Updated llvm versions for linux dylib lookup as tests weren't passing
HosseinYousefi pushed a commit that referenced this pull request Nov 16, 2023
@dcharkes
[native_assets_cli] add packageName (#146)
64cd7ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HosseinYousefi HosseinYousefi HosseinYousefi approved these changes

Assignees
No one assigned
Labels
package:native_assets_cli
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dcharkes @coveralls @j4qfrost @HosseinYousefi