Rename NativeCodeAsset->CodeAsset and merge {CodeAsset,DataAsset} and {CodeAsset,DataAsset}Impl #1614

mkustermann · 2024-09-27T19:52:45Z

This is the last base refactoring. Building on this the next PRs will gradually introduce more flexibility into the build system (starting with adding custom asset types) and gradually separate components more and more.

Some of the class relationships and methods in this PR will soon disappear again, but for ease of reviewing
it may make sense to send this piece out individually.

The PR

Renames NativeCodeAsset to CodeAsset (which is easier to type and more similar to DataAsset)
Unifies CodeAsset and CodeAssetImpl, this also aligns their constructors regarding whether they take an id or a package and name
Unifies DataAsset and DataAssetImpl

… {CodeAsset,DataAsset}Impl This is the last base refactoring. Building on this the next PRs will gradually introduce more flexibility into the build system (starting with adding custom asset types) and gradually seperate components more and more.

github-actions · 2024-09-27T19:54:30Z

PR Health

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
native_assets_cli	Breaking	0.9.0-wip	0.9.0-wip	0.9.0-wip	✔️

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package	Leaked API symbols

License Headers ✔️

// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers

Files
pkgs/ffigen/example/libclang-example/generated_bindings.dart
pkgs/ffigen/example/shared_bindings/generate.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_shared_b_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/base_gen.dart
pkgs/ffigen/example/simple/generated_bindings.dart
pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_decl_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_symbol_address_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_type_name_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_reserved_keyword_collision_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_comment_markup_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_dart_handle_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_enum_int_mimic_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_forward_decl_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_functions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_imported_types_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_native_func_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_opaque_dependencies_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_packed_structs_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_regress_384_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_struct_fptr_fields_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_unions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_varargs_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_cjson_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_libclang_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_sqlite_bindings.dart
pkgs/ffigen/test/native_test/_expected_native_test_bindings.dart
pkgs/jni/lib/src/third_party/generated_bindings.dart
pkgs/jni/lib/src/third_party/global_env_extensions.dart
pkgs/jni/lib/src/third_party/jni_bindings_generated.dart
pkgs/jnigen/android_test_runner/lib/main.dart
pkgs/jnigen/example/in_app_java/lib/android_utils.dart
pkgs/jnigen/example/kotlin_plugin/example/lib/main.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_bindings.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/pdfbox_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocument.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocumentInformation.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/_package.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/PDFTextStripper.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/_package.dart
pkgs/jnigen/lib/src/bindings/descriptor.dart
pkgs/jnigen/lib/src/elements/elements.g.dart
pkgs/jnigen/test/jackson_core_test/third_party/bindings/com/fasterxml/jackson/core/_package.dart
pkgs/jnigen/tool/command_runner.dart
pkgs/native_assets_builder/test_data/native_dynamic_linking/bin/native_dynamic_linking.dart
pkgs/swift2objc/lib/src/config.dart
pkgs/swift2objc/lib/src/generate_wrapper.dart
pkgs/swift2objc/lib/src/generator/_core/utils.dart
pkgs/swift2objc/lib/src/generator/generator.dart
pkgs/swift2objc/lib/src/generator/generators/class_generator.dart
pkgs/swift2objc/lib/src/parser/parsers/declaration_parsers/parse_initializer_declaration.dart
pkgs/swift2objc/lib/src/parser/parsers/declaration_parsers/parse_property_declaration.dart
pkgs/swift2objc/lib/src/transformer/_core/unique_namer.dart
pkgs/swift2objc/lib/src/transformer/_core/utils.dart
pkgs/swift2objc/lib/src/transformer/transformers/transform_property.dart
pkgs/swift2objc/test/unit/parse_initializer_param_test.dart

Package publish validation ✔️

Package	Version	Status
package:ffi	2.1.3	already published at pub.dev
package:ffigen	15.0.0-wip	WIP (no publish necessary)
package:jni	0.12.0-wip	WIP (no publish necessary)
package:jnigen	0.12.0-wip	WIP (no publish necessary)
package:native_assets_cli	0.9.0-wip	WIP (no publish necessary)
package:objective_c	2.1.0-wip	WIP (no publish necessary)
package:swift2objc	0.0.1-wip	WIP (no publish necessary)
package:swiftgen	0.0.1-wip	WIP (no publish necessary)

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

coveralls · 2024-09-27T20:01:54Z

coverage: 92.178% (-0.04%) from 92.22%
when pulling a2e31ed on data-code-impl
into 1aae9da on main.

dcharkes

Renames NativeCodeAsset to CodeAsset (which is easier to type and more similar to DataAsset)

Technically speaking a JarAsset is also a code asset. How can we say that this is code compiled to a static/dynamic library? I wonder if NativeLibrary would be a better name. But, it doesn't have Asset in the name.

The rest of the code changes lgtm.

pkgs/native_assets_cli/lib/src/json_utils.dart

pkgs/native_assets_cli/lib/src/api/code_asset.dart

pkgs/native_assets_cli/CHANGELOG.md

mkustermann · 2024-09-30T08:46:13Z

Technically speaking a JarAsset is also a code asset. How can we say that this is code compiled to a static/dynamic library? I wonder if NativeLibrary would be a better name. But, it doesn't have Asset in the name.

Yes. I'd assume we later on may add WasmAsset, JarAsset, ... and yes maybe it would make sense to call shared libraries e.g. SharedLibraryAsset.

We may even leave the "asset" eventually away before we declare 1.0 - e.g. a hook author could just write output.jars.add() or output.sharedLibraries.add() ...

If you feel strongly about it I can rename it back, but I think this makes all of related code just read so much nicer.

…g,...

mkustermann · 2024-09-30T09:20:02Z

I think only the question around NativeCodeAsset -> CodeAsset rename remains. @dcharkes

dcharkes · 2024-09-30T09:50:50Z

I think only the question around NativeCodeAsset -> CodeAsset rename remains. @dcharkes

If we think we want to rename it later, maybe not rename it now. It will require migrations in existing hooks. I don't feel very strongly about it though. Since you're doing the dartdev and flutter_tools ones. 😄

mkustermann · 2024-10-01T07:17:36Z

If we think we want to rename it later, maybe not rename it now. It will require migrations in existing hooks. I don't feel very strongly about it though. Since you're doing the dartdev and flutter_tools ones. 😄

If you don't feel very strongly about it, I'll keep it like this - as I think it makes code dealing with it read much nicer.

github-actions bot added package:native_assets_cli package:native_toolchain_c package:native_assets_builder labels Sep 27, 2024

dcharkes reviewed Sep 30, 2024

View reviewed changes

mkustermann added 2 commits September 30, 2024 10:59

Move src/{api,->}/{code_asset,data_asset,asset}.dart, update changelo…

cab5892

…g,...

object -> map$, optionalObject -> optionalMap

a2e31ed

dcharkes approved these changes Sep 30, 2024

View reviewed changes

mkustermann merged commit d144f81 into main Oct 1, 2024
32 checks passed

mkustermann deleted the data-code-impl branch October 1, 2024 07:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename NativeCodeAsset->CodeAsset and merge {CodeAsset,DataAsset} and {CodeAsset,DataAsset}Impl #1614

Rename NativeCodeAsset->CodeAsset and merge {CodeAsset,DataAsset} and {CodeAsset,DataAsset}Impl #1614

mkustermann commented Sep 27, 2024 •

edited

Loading

github-actions bot commented Sep 27, 2024

coveralls commented Sep 27, 2024 •

edited

Loading

dcharkes left a comment

mkustermann commented Sep 30, 2024

mkustermann commented Sep 30, 2024

dcharkes commented Sep 30, 2024

mkustermann commented Oct 1, 2024

Rename NativeCodeAsset->CodeAsset and merge {CodeAsset,DataAsset} and {CodeAsset,DataAsset}Impl #1614

Rename NativeCodeAsset->CodeAsset and merge {CodeAsset,DataAsset} and {CodeAsset,DataAsset}Impl #1614

Conversation

mkustermann commented Sep 27, 2024 • edited Loading

github-actions bot commented Sep 27, 2024

PR Health

coveralls commented Sep 27, 2024 • edited Loading

dcharkes left a comment

Choose a reason for hiding this comment

mkustermann commented Sep 30, 2024

mkustermann commented Sep 30, 2024

dcharkes commented Sep 30, 2024

mkustermann commented Oct 1, 2024

mkustermann commented Sep 27, 2024 •

edited

Loading

coveralls commented Sep 27, 2024 •

edited

Loading