Finished API review of all repos

5313c017 · Alexandru Croitor · 5a80b229 · 5313c017
Commit 5313c017 authored Jul 14, 2021 by Alexandru Croitor
--- a/api_review_qt_6_2.md
+++ b/api_review_qt_6_2.md
@@ -149,8 +149,10 @@ function(qt6_add_executable target)
    cmake_parse_arguments(PARSE_ARGV 1 arg "MANUAL_FINALIZATION" "" "")
 ```

-`qt6_finalize_target` - new API
+**Decision:**
+All is fine.

+`qt6_finalize_target` - new API

 `qt_finalize_target`

@@ -158,6 +160,9 @@ function(qt6_add_executable target)
 function(qt6_finalize_target target)
 ```

+**Decision:**
+All is fine here.
+
 `qt_finalize_executable` - existing API kept for QtCreator compatibility

 ```cmake
@@ -166,6 +171,10 @@ function(qt_finalize_executable)
 endfunction()
 ```

+**Decision:**
+All is fine here.
+
+
 `qt6_enable_import_plugins_finalizer_mode` - new API

 `qt_enable_import_plugins_finalizer_mode`
@@ -174,6 +183,10 @@ endfunction()
 function(qt6_enable_import_plugins_finalizer_mode target enabled)
 ```

+**Decision:**
+AP: Combine qt6_enable_import_plugins_finalizer_mode and qt6_enable_object_libraries_finalizer_mode into single API call that takes
+a CONTEXT keyword, so we have a single API to enable various kinds of finalizer modes.
+
 `qt6_enable_object_libraries_finalizer_mode` - new API

 `qt_enable_object_libraries_finalizer_mode`
@@ -182,6 +195,9 @@ function(qt6_enable_import_plugins_finalizer_mode target enabled)
 function(qt6_enable_object_libraries_finalizer_mode target enabled)
 ```

+**Decision:**
+AP: Wait until 3.21.0 is released (hopefully very soon) so we can remove this one.
+
 `qt6_add_plugin` - existing TP API

 `qt_add_plugin`
@@ -199,6 +215,10 @@ function(qt6_add_plugin target)
    )
 ```

+**Decision:**
+AP: Rename `TYPE` to `QT_PLUGIN_TYPE` because it's a bit too generic.
+AP: Ensure documenting that if neither STATIC or SHARED is specified, the default is not like the CMake behavior, but rather what Qt was configured as (STATIC or SHARED).
+
 `qt6_add_library` - new API

 `qt_add_library`
@@ -208,6 +228,11 @@ function(qt6_add_library target)
    cmake_parse_arguments(PARSE_ARGV 1 arg "MANUAL_FINALIZATION" "" "")
 ```

+**Decision:**
+All is fine.
+AP: Create JIRA task to replace uses of add_library with qt_add_library in the implementations of our functions where it makes sense (e.g qt_add_qml_module).
+
+
 `qt6_wasm_add_target_helpers` - new API, probably should be internal

 `qt_wasm_add_target_helpers`
@@ -216,6 +241,10 @@ function(qt6_add_library target)
 function(qt6_wasm_add_target_helpers target)
 ```

+**Decision:**
+AP: Make the function internal, not public API. Rename it as well.
+
+
 ## qtshadertools

 `qt6_add_shaders` - existing API
@@ -228,6 +257,8 @@ function(qt6_add_shaders)
 endfunction()
 ```

+**Decision:**
+
 `qt6_add_shaders_impl` - new API, probably needs to be `_qt_internal_add_shaders_impl`

 New options are `INTERNAL`, `SILENT` , `OUTPUTS`
@@ -243,6 +274,10 @@ function(qt6_add_shaders_impl target resourcename)
    )
 ```

+**Decision:**
+AP: Document `SILENT` and `OUTPUTS`. Rename the functions to be internal.
+AP: Consider renaming `SILENT` to `QUIET`.
+
 `qt6_internal_add_shaders` - new API, probably needs to be removed

 `qt_internal_add_shaders` - new API, probably needs to be `_qt_internal_add_shaders`
@@ -253,6 +288,8 @@ function(qt6_internal_add_shaders)
 endfunction()
 ```

+**Decision:**
+AP: Rename the functions to be internal.

 ## qtremoteobjects

@@ -262,6 +299,11 @@ Missing versionless API
 `qt6_add_repc_replicas` renamed from `qt6_add_repc_replica`
 Missing versionless API

+**Decision:**
+AP: Keep the old names for compatibility, warn old names as deprecated.
+AP: qt6_add_repc_files should have been internal, but unfortunately already public API, warn not to use it, and use one of
+`qt6_add_repc_sources`, `qt6_add_repc_replicas`, `qt6_add_repc_merged` instead.
+AP: Remove all the old names from docs.

 ## qtwayland

@@ -274,6 +316,8 @@ function(qt6_generate_wayland_protocol_client_sources target)
    cmake_parse_arguments(arg "" "WAYLAND_INCLUDE_DIR" "FILES" ${ARGN})
 ```

+**Decision:**
+
 `qt6_generate_wayland_protocol_server_sources` - existing API

 New option is `WAYLAND_INCLUDE_DIR`
@@ -283,6 +327,8 @@ function(qt6_generate_wayland_protocol_server_sources target)
    cmake_parse_arguments(arg "" "WAYLAND_INCLUDE_DIR" "FILES" ${ARGN})
 ```

+**Decision:**
+AP: Ask if `WAYLAND_INCLUDE_DIR` is meant to be an internal option or it's only needed for Qt.

 ## qtscxml

@@ -299,6 +345,10 @@ function(qt6_add_statecharts target_or_outfiles)
    cmake_parse_arguments(ARGS "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
 ```

+**Decision:**
+AP: Keep `OPTIONS` as the proper name, remove the old one. We can re-add it later if we really need it.
+AP: Rename `OUTPUT_DIR` to `OUTPUT_DIRECTORY`.
+
 ## qtquick3d

 `qt6_quick3d_bake_lightprobe_hdri` - new API
@@ -326,11 +376,18 @@ function(qt6_quick3d_bake_lightprobe_hdri target resource_name)
    )
 ```

+AP: Don't document `INTERNAL` option.
+AP: Clarify name with quick3d team, does it need `quick3d` infix in the function name, could it be renamed to a more generic name for future extension.
+
+**Decision:**
+
 `qt6_quick3d_internal_bake_lightprobe_hdri` - new API, wraps qt6_quick3d_bake_lightprobe_hdri

 `qt_quick3d_internal_bake_lightprobe_hdri`

+**Decision:**

+AP: Make it an internal function, not a public API.
 ## qttools

 `qt6_add_lupdate` - new API
@@ -350,6 +407,9 @@ function(qt6_add_lupdate target)
    cmake_parse_arguments(arg "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
 ```

+**Decision:**
+All is fine here.
+
 `qt6_add_lrelease` - new API

 `qt_add_lrelease`
@@ -366,6 +426,16 @@ function(qt6_add_lrelease target)
        OPTIONS)
    cmake_parse_arguments(arg "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
 ```
+```cmake
+    if(NOT arg_MANUAL)
+        add_dependencies(${target} ${target}_lrelease)
+    endif()
+```
+Proposal: Rename `MANUAL` to `NO_TARGET_DEPENDENCY`
+AP: Do proposal above.
+
+
+**Decision:**

 `qt6_add_translations` - new API

@@ -375,7 +445,9 @@ function(qt6_add_lrelease target)
 function(qt6_add_translations target)
    set(options)
    set(oneValueArgs
-        QM_FILES_OUTPUT_VARIABLE)
+        QM_FILES_OUTPUT_VARIABLE
+        RESOURCE_PREFIX
+        OUTPUT_TARGETS)
    set(multiValueArgs
        TS_FILES
        SOURCES
@@ -385,6 +457,30 @@ function(qt6_add_translations target)
    cmake_parse_arguments(arg "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
 ```

+    qt6_add_translations(my_app
+        TS_FILES my_app_de.ts my_app_no.ts
+        RESOURCE_PREFIX "/translations")
+
+```qmake
+lconvert
+TRANSLATION = foo_de.ts foo_ru.ts
+lupdate myproject.pro
+```
+
+**Decision:**
+Proposal:
+* If RESOURCE_PREFIX is not specified, by default a create a resource with a prefix set to '/i18n' to match the resource prefix picked up
+magically by QQmlApplicationEngine.
+* Users can change the prefix to one they would like instead of the default.
+* If QM_FILES_OUTPUT_VARIABLE is specified, a resource is NOT automatically created anymore, because the files are intended to be installed
+instead.
+* If both QM_FILES_OUTPUT_VARIABLE and RESOURCE_PREFIX are specified, issue FATAL_ERROR.
+AP: Go ahead with proposal above.
+AP: Mark qt6_add_translations as Technical Preview for 6.2, qt6_add_lrelease and qt6_add_lupdate are released fully.
+    Reason: We don't feel comfortable committing to this API because we don't know how the project-wide translations use case might affect it.
+AP: Add OUTPUT_TARGETS as an out_var for when qt6_add_translations is called on a static library target.
+AP: Craig: Check if MANUAL add_dependency call causes an always out of date target (depends on implementation).
+AP: Create Jira task to research how to handle project wide translations.

 ## qtactiveqt

@@ -417,6 +513,8 @@ function(qt6_target_typelibs target)
    cmake_parse_arguments(arg "COMPAT" "OUTPUT_DIRECTORY" "LIBRARIES" ${ARGN})
 ```

+**Decision:**
+All is fine.

 `qt6_add_axserver_executable` - new API

@@ -431,6 +529,9 @@ function(qt6_add_axserver_executable target)
    cmake_parse_arguments(arg "SKIP_AX_SERVER_REGISTRATION" "" "" ${ARGN})
 ```

+**Decision:**
+AP: Change `SKIP_AX_SERVER_REGISTRATION` to `NO_AX_SERVER_REGISTRATION`.
+AP: Craig: Check implementation of qt6_add_axserver_executable, the boolean option passed to qt6_target_idl might not be passed correctly.

 `qt6_add_axserver_library` - new API

@@ -445,6 +546,9 @@ function(qt6_add_axserver_library target)
    cmake_parse_arguments(arg "SKIP_AX_SERVER_REGISTRATION" "" "" ${ARGN})
 ```

+**Decision:**
+AP: Change `SKIP_AX_SERVER_REGISTRATION` to `NO_AX_SERVER_REGISTRATION`.
+
 `qt6_target_idl` - new API

 `qt_target_idl`
@@ -461,6 +565,9 @@ function(qt6_target_idl target)
    cmake_parse_arguments(arg "SKIP_AX_SERVER_REGISTRATION" "" "" ${ARGN})
 ```

+**Decision:**
+AP: Change `SKIP_AX_SERVER_REGISTRATION` to `NO_AX_SERVER_REGISTRATION`.
+AP: Change name of global variable too from `QT_SKIP_AX_SERVER_REGISTRATION` to `QT_NO_AX_SERVER_REGISTRATION`.

 ## qtdeclarative