[FEATURE] Versioninfo enrich with manifest infos #554

tobiasso85 · 2020-11-27T14:58:50Z

Enrich task generateVersionInfo, such that the created version-info.json file contains new sections in the json structure:

manifestHints: for each library (in each library's json structure)
components: global (top level in the json structure)

manifestHints

The manifestHints section contains the libraries dependencies (fully resolved).

The resolved dependencies are

lazy
- if parent dependencies are also lazy
non-lazy
- if lazy is not defined for this dependency and it has no lazy parent
- if there is the same dependency defined lazy but the current one isn't (non-lazy wins on the same level)

sample:

{
  "name": "sap.x",
  "version": "1.85.0",
  "manifestHints": {
    "dependencies": {
      "libs": {
        "sap.m": { },
        "sap.ui.core": { },
        "sap.ui.layout": { },
        "sap.ui.unified": {
          "lazy": true
        }
      }
    }
  }
},

components

The components section contains all components which are embedded by the libraries.
Each component has:

the including library
manifestHints
hasOwnPreload. It indicates that embeds points to subcomponents and embeddedBy points to parent components

sample:

  "components": {
    "sap.x.sub": {
      "library": "sap.x",
      "manifestHints": {
        "dependencies": {
          "libs": {
            "sap.m": {},
            "sap.ui.core": {},
            "sap.ui.layout": {},
            "sap.ui.unified": {
              "lazy": true
            }
          }
        }
      }
    },

codeworrior · 2020-12-02T07:22:28Z

lib/processors/versionInfoGenerator.js

+		return resolvedCache.get(libName);
+	}
+	const manifestHint = manifestHints.get(libName); // lib.c
+	console.log(`:processing: ${libName}`);


remove/rewrite to logger before final submit

codeworrior · 2020-12-02T07:36:45Z

lib/processors/versionInfoGenerator.js

+	const manifestContent = await manifestResource.getString();
+	const manifestObject = JSON.parse(manifestContent);
+	// TODO extract manifestHints
+	const manifestDependencies = manifestObject["sap.ui5"]["dependencies"];


Existence of a "sap.ui5" entry is not guaranteed

codeworrior · 2020-12-02T07:41:18Z

lib/processors/versionInfoGenerator.js

+ *
+ * @example
+ * {
+ *  sap.chart: {


This is not an example for the above typedef (incomplete)

codeworrior · 2020-12-02T07:42:47Z

lib/processors/versionInfoGenerator.js

+		return filePath.substr(0, filePath.length - "manifest.json".length) + subPath + "/manifest.json";
+	}
+	return filePath;
+};


Better use something like path.posix.resolve. See manifestCreator.

Use the full power of posixPath:

return posixPath.resolve(posixPath.dirname(filePath), subPath, "manifest.json");

codeworrior · 2020-12-02T07:46:10Z

lib/processors/versionInfoGenerator.js

+	if (existingEntry) {
+		Object.keys(existingEntry).forEach((libName) => {
+			if (!existingEntry[libName].lazy && newLibs[libName] && newLibs[libName].lazy) {
+				newLibs[libName] = {};


Essentially, this is meant to be a delete newLibs[libName].lazy. Maybe better write it as this. If we ever add other information per library object (e.g. minVersion), this line doesn't cause extra work.

If you want to avoid the delete, set the property to undefined. Later serialisation to JSON will omit the undefined properties.

codeworrior · 2020-12-02T07:48:53Z

lib/processors/versionInfoGenerator.js

+ */
+const setManifestHints = (manifestHints, libName, newObject) => {
+	const existingEntry = manifestHints.get(libName);
+	const newLibs = merge(existingEntry && existingEntry, newObject);


codeworrior · 2020-12-02T07:49:17Z

lib/processors/versionInfoGenerator.js

+};
+
+const merge = (existingEntry, newObject) => {
+	const newLibs = clone(newObject);


Why is the clone needed? As far as I can see, the returned value replaces existingEntry, at least in setManifestHintsas well as in line 167.

codeworrior · 2020-12-02T07:53:46Z

lib/processors/versionInfoGenerator.js

@@ -33,20 +203,121 @@ module.exports = async function({options}) {
 	}

 	const buildTimestamp = getTimestamp();
+	// TODO filter manifest.json if sap/embeds (we expect it contains the correct information)


sap.app/embeds?

codeworrior · 2020-12-02T07:56:12Z

lib/processors/versionInfoGenerator.js

+	});
+
+	// TODO sort!
+	embeddedInfoMap.forEach((embeddedInfo, libName) => {


Aren't the keys in the components object component names, not library names?

codeworrior · 2020-12-02T08:03:30Z

lib/tasks/generateVersionInfo.js

+	// build with --all should not use the version-info.json
+	// logic needs to be adjusted once selective dependencies are built
+	// TODO: transient resources (not part of the build result) ( -> skipped for now)
+	// exclude task if not build --all


redundant to comment in builder.js

coveralls · 2021-01-04T12:58:01Z

Coverage increased (+0.1%) to 93.835% when pulling daf5610 on versioninfo-enrich-with-manifest-infos into 31a63d9 on master.

codeworrior · 2021-01-14T10:29:29Z

lib/processors/versionInfoGenerator.js

+ *
+ * @typedef {object<string, object>} ManifestLibraries
+ *
+ *  * @example


[minor,jsdoc] one asterisk too much

tackled with 1fe9c25

codeworrior · 2021-01-14T10:39:37Z

lib/processors/versionInfoGenerator.js

+ * @property {string} name The library name
+ * @property {string} version The library version
+ * @property {module:@ui5/fs.Resource} libraryManifest main manifest resources
+ * @property {module:@ui5/fs.Resource[]} manifestResources list of corresponding manifest resources


What does 'corresponding' mean here? Those found within in the library, but not including the libraryManifest itself?

tackled with ea58d08

I would separate them and make libraryManifest and embeddedManifest (not including the libraryManifest), this is still open

manifestResources do not contain the libraryManifest anymore(874b160)

codeworrior · 2021-01-14T10:42:35Z

lib/processors/versionInfoGenerator.js

+	const embeddedPaths = embeds.map((embed) => {
+		return getManifestPath(libraryInfo.libraryManifest.getPath(), embed);
+	});
+	// sap.x.sdk


sap.x.sdk ???

tackled with ea58d08

codeworrior · 2021-01-14T12:14:19Z

lib/processors/versionInfoGenerator.js

+/**
+ * Library Info
+ *
+ * contains information about the name the version of the library and its manifest, as well as the nested manifests.


... the name and version ...

tackled with ea58d08

codeworrior · 2021-01-14T12:15:07Z

lib/processors/versionInfoGenerator.js

+ *      libraryManifest: module:@ui5/fs.Resource,
+ *      manifestResources: module:@ui5/fs.Resource[]
+ *   }
+ * </code>


<code> tags alone don't preserve the formatting, better use <pre>

tackled with ea58d08

codeworrior · 2021-01-14T12:32:14Z

lib/processors/versionInfoGenerator.js

+ *
+ * @param {Map<string, DependencyInfo>} dependencyInfoMap
+ */
+const resolveTransitiveDependencies = (dependencyInfoMap) => {


I find the variable name libraryInfo misleading. In the current implementation, dependencyInfoMap contains information about libraries as well as embedded components.

But I'm also not sure if this is the way how the Java code works. AFAIK, Maven just cares about the library dependencies. And it first resolves libraries and then resolves the library dependencies of components.

At the end, the result might be the same as we don't add component dependencies to the DependencyInfo objects, but if someone mixes up lib and component dependencies, then the code here won't notice whereas Maven would complain.

in maven the dependencyInfoMap also contains both

But first the library dependencies get resolved, then the components get resolved

separated with 1fe9c25

codeworrior · 2021-01-14T12:49:32Z

lib/processors/versionInfoGenerator.js

+	resolve(dependencyInfoMap, lazy) {
+		if (!this.wasResolved || lazy) {
+			this.libs.forEach((depInfoObject) => {
+				const dependencyInfoObjectAdded = this.addResolvedLibDependency(depInfoObject.name, depInfoObject.lazy);


I'm missing a use of the local lazy here. When the incoming dependency is lazy, even eager dependencies of a component or lib get lazy dependencies of the caller.

The OR in line 254 does something like this, but I wonder why the lazy is needed then.

tackled with f14cba5

codeworrior · 2021-01-14T13:25:59Z

test/lib/tasks/generateVersionInfo.js

+					}
+				}
+			},
+			"name": "lib.a",


would you mind to make the name the first property? That makes it easier to understand.

tackled with 874b160

codeworrior · 2021-01-14T13:35:42Z

lib/processors/versionInfoGenerator.js

+	 *  all its dependencies should be treated as lazy
+	 */
+	resolve(dependencyInfoMap, lazy) {
+		if (!this.wasResolved || lazy) {


The resolution is kept as state of this instance. It is only updated when lazy is true.
But what when resolve is called again afterwards with lazy = false? Then the result will be the one from the last resolution with lazy = true - which is wrong.

tackled with f14cba5

codeworrior · 2021-01-14T13:37:41Z

lib/processors/versionInfoGenerator.js

+/**
+ * Extracted information from a manifest's <code>sap.app</code> and <code>sap.ui5</code> sections.
+ */
+class ManifestInfo {


IMO, the ManifestInfo is not of much use in its current implementation. it just requires copying back and forth data and brings no added value.

used typedef instead with 1fe9c25

improve code by passing correct arguments

adjusted logic to resolve dependencies

adjusted logic to resolve dependencies correctly with regards to lazy

added logic to use info for embeds

improved components structure

minor adjustments

simplified merge logic

improved test sort keys of objects

adjusted jsdoc of libraryInfos

added TODOs

refactoring of dependency resolution logic

fix recursive logic such that lazy property correctly gets interpreted: * dependencies of lazy dependency are also lazy * having the same dependency lazy and non-lazy results in a non-lazy dependency

Rename types to avoid confusion

Rename variable to componentName because it is not the library name

Support embeddedBy property of subcomponents. hasOwnPreload is set if child and parent component point to each other via embeds and embeddedBy properties. * the parent component uses embeds to point to its children * the child component uses embeddedBy to point to its parent both paths are relative - AppIndex expects all nested components to be listed in the "sap.app"/"embeds" array of a library's manifest.json, no matter whether the corresponding component is bundled with the library or in an individual Component-preload bundle - UI5 runtime needs to know whether a component is bundled as part of the library-preload of the containing library. Up to now, it assumed this to be the case when the component is listed in sap-ui-version.json - as these two requirements don't match, a new flag (hasOwnPreload) is introduced per component in the sap-ui-version.json which indicates whether the component has an own bundle or not The value of the flag is based on the 'embeddedBy' property as declared in the component's manifest. It is not based on build configuration. This is in line with the AppIndex's implementation.

Leaving out optional parameters like libraryManifest and manifestResources must produce the same result as before

use typedef instead of own class for a data container do embedded component dependencies resolving after library dependencies resolving in separate step to avoid mixing them up.

use tag <pre> in jsdoc to preserve formatting Use the same sample across the code e.g. lib.x.sub

manifests and not the library manifest anymore in the tests adjust expected version info content the same order of properties as the json string result would contain for better readability.

compare objects and omit the exact buildTimestamp

but not in project dependencies improve recursive resolve logic improve jsdoc for ArtifactInfo use better structures instead of classes for * libs and libsResolved * DependencyInfoObject Simplify tests

refactor duplicate code to common method processManifestAndGetArtifactInfo

add tests for versionInfoGenerator with new parameters * libraryManifest * embeddedManifests

codeworrior

Basically LGTM. Comments are about potential improvements (opinions about whether those are really improvments might even differ).

Only the point about lazy:false might be worth looking at before a submit.

codeworrior · 2021-01-21T14:43:58Z

lib/processors/versionInfoGenerator.js

+			this.libsResolved[libName] = alreadyResolved;
+		} else {
+			// siblings if sibling is eager only if one other sibling eager
+			alreadyResolved.lazy = alreadyResolved.lazy && lazy;


I know I made the proposal to write this as a single line. But now that the libsResolved are used 1:1 for the manifestHints, this has the disadvantage that a lazy:false might appear in the final output, which is unexpected.

this is already handled by:
https://github.com/SAP/ui5-builder/pull/554/files/b9da72db5dec100900c85b27ce69c2cee73a1a88#diff-5fc9387f7e46413dbf8c8988ee00af7a121520ae98e152f8c107d782a77c1ee6R65
it can only be true or it does not exist

Not only by that line, but with the help of several others and by the nature of the && operator and by JSON.stringify which doesn't stringify an undefined value...

But you're right. I indeed wondered why the tests didn't show any unexpected lazy:false values but was too lazy to investigate this.

I would leave it untouched then.

codeworrior · 2021-01-21T14:47:10Z

lib/processors/versionInfoGenerator.js

+	 * @param {Map<string,DependencyInfo>} dependencyInfoMap
+	 */
+	resolve(dependencyInfoMap) {
+		if (!this.wasResolved) {


[Potential simplification] You could use the existence of libsResolved as marker instead of yet another property.

And by making this method work like a getter (return this.libsResolved), the calling code would not have to operate on an internal property of dependencyInfo.

And, by making it a getter, even the resolve loops could disappear in the main method. Calling the getter would be enough to trigger all the necessary calculations.

tackled with fd6a8e9
also made libsResolved field "private" by letting it start with underscore

codeworrior · 2021-01-21T14:53:17Z

lib/processors/versionInfoGenerator.js

+					dependencyInfo.resolve(dependencyInfoMap);
+
+					// children if parent is lazy children become lazy
+					Object.keys(dependencyInfo.libsResolved).forEach((resolvedLibName) => {


[Potential simplification] This code pattern

Object.keys(map).forEach((key) => { const value = map[key]; ... });

occurs several times and (IMHO) reduces readability of the code.
What about

for (const [key, value] of Object.entries(map)) { ... }

tackled with fd6a8e9

codeworrior · 2021-01-21T14:57:06Z

lib/processors/versionInfoGenerator.js

+
+
+	// process library infos
+	const librariesPromises = options.libraryInfos.map((libraryInfo) => {


libraryPromises, similar to libraryInfos?

you mean the name?
i would change the name to libraryInfosProcessPromises or artifactInfosPromises

tackled with fd6a8e9
renamed to libraryInfosProcessPromises

codeworrior · 2021-01-21T14:59:37Z

lib/processors/versionInfoGenerator.js

+			embeddedArtifactInfo.dependencyInfo.resolve(dependencyInfoMap);
+		});
+	});
+


Lines 449 ... 459 could be omitted when resolve would be implemented as a getter for libsResolved(and when that getter would be called by getManifestHints).

Only drawback would be that getManifestHints needs the dependencyInfoMap as parameter.

tackled with fd6a8e9

codeworrior · 2021-01-21T15:09:55Z

lib/processors/versionInfoGenerator.js

+	artifactInfos.forEach((artifactInfo) => {
+		artifactInfo.getEmbeds().forEach((embeddedArtifactInfo) => {
+			const componentObject = {
+				library: embeddedArtifactInfo.parentComponentName


can embeddedArtifactInfo.parentComponentName ever differ from artifactInfo.name?

tackled with fd6a8e9
Put ArtifactInfo as typedef because the parent fields in the child could be retrieved by directly using the parent's fields.

use typedef for ArtifactInfo instead of class because functionality could be extracted and it served then only as a data container. replaced Object.keys().forEach loops which Object.entries loop Use getResolvedLibraries for directly resolving transitive dependencies

codeworrior · 2021-01-26T09:01:23Z

lib/processors/versionInfoGenerator.js

+		if (manifestDependencies) {
+			const libs = {};
+			for (const [libKey, libValue] of Object.entries(manifestDependencies.libs)) {
+				libs[libKey] = {};


Iteration fails for manifestDependencies.libs === undefined (or null). This was already an issue of the older implementation (using Object.keys). Maybe enrich the if with a second term.

tackled with 8eeab50

codeworrior · 2021-01-26T09:06:58Z

lib/processors/versionInfoGenerator.js

+		return filePath.substr(0, filePath.length - "manifest.json".length) + subPath + "/manifest.json";
+	}
+	return filePath;
+};


Use the full power of posixPath:

return posixPath.resolve(posixPath.dirname(filePath), subPath, "manifest.json");

codeworrior · 2021-01-26T09:17:59Z

lib/processors/versionInfoGenerator.js

@@ -14,6 +16,335 @@ function getTimestamp() {
 	return year + month + day + hours + minutes;


I don't remember what we said about timestamps. UTC or not?

handled in separate PR
#348

simplify manifest path resolving

codeworrior

LGTM.

RandomByte · 2021-01-26T16:45:11Z

lib/processors/versionInfoGenerator.js

+const processLibraryInfo = async (libraryInfo) => {
+	if (!libraryInfo.libraryManifest) {
+		log.warn(
+			`Cannot add meta information for library '${libraryInfo.name}'. The manifest.json file cannot be found`);


When executing a ui5 build without --all, this warning appears for all framework dependencies since no manifest.json files are generated for them.

I guess we already discussed that this task doesn't really make sense when executed without --all (hence the new comment in builder.js). But since it is executed in such builds as of today, I find the warning irritating and think it should not appear in that case.

Maybe we need to check whether the has not been supplied at all?

Example from the openui5-sample-app:

> ui5 build info normalizer:translators:ui5Framework Using OpenUI5 version: 1.85.2 info builder:builder Building project openui5-sample-app not including dependencies... info builder:builder 🛠 (1/1) Building project openui5-sample-app info builder:builder application openui5-sample-app 🔨 (1/8) Running task escapeNonAsciiCharacters... info builder:builder application openui5-sample-app 🔨 (2/8) Running task replaceCopyright... info builder:builder application openui5-sample-app 🔨 (3/8) Running task replaceVersion... info builder:builder application openui5-sample-app 🔨 (4/8) Running task generateFlexChangesBundle... info builder:builder application openui5-sample-app 🔨 (5/8) Running task generateComponentPreload... info builder:builder application openui5-sample-app 🔨 (6/8) Running task createDebugFiles... info builder:builder application openui5-sample-app 🔨 (7/8) Running task uglify... info builder:builder application openui5-sample-app 🔨 (8/8) Running task generateVersionInfo... WARN builder:processors:versionInfoGenerator Cannot add meta information for library 'sap.ui.unified'. The manifest.json file cannot be found WARN builder:processors:versionInfoGenerator Cannot add meta information for library 'sap.m'. The manifest.json file cannot be found WARN builder:processors:versionInfoGenerator Cannot add meta information for library 'sap.ui.core'. The manifest.json file cannot be found WARN builder:processors:versionInfoGenerator Cannot add meta information for library 'sap.ui.layout'. The manifest.json file cannot be found WARN builder:processors:versionInfoGenerator Cannot add meta information for library 'sap.f'. The manifest.json file cannot be found info builder:builder Build succeeded in 574 ms info builder:builder Executing cleanup tasks...

Without this change, no such warning appears. The application can't resolve this warning, hence it should not be shown to the application developer.

tackled with 42ce003
the log is verbose now and does not occur anymore when building the openui5 sample app without --all

RandomByte · 2021-01-26T16:54:34Z

lib/processors/versionInfoGenerator.js

- * @param {Array} parameters.options.libraryInfos Array of objects representing libraries,
- *					e.g. <code>{name: "library.xy", version: "1.0.0"}</code>
- * @returns {Promise<module:@ui5/fs.Resource[]>} Promise resolving with an array containing the versioninfo resource
+ * @param {LibraryInfo[]} parameters.options.libraryInfos Array of objects representing libraries,


The LibraryInfo type definition needs to be public as well to be visible in the JSDoc

tackled with daf5610

With regards to backward compatibility the libraryManifest can be missing. When --all is omitted the libraryManifest can also be missing.

It is used in the versionInfoGenerator's public JSDoc.

KlattG · 2021-01-28T10:09:16Z

lib/processors/versionInfoGenerator.js

+
+	/**
+	 * Resolves dependencies recursively and retrieves them with
+	 * - resolved siblings a lazy and a eager dependency becomes eager


I do not understand this line. Is there anything missing?

KlattG · 2021-01-28T10:10:05Z

lib/processors/versionInfoGenerator.js

+
+/**
+ * Common type for Library and Component
+ * embeds and bundled components makes only sense for library


KlattG · 2021-01-28T10:10:27Z

lib/processors/versionInfoGenerator.js

+ *
+ * @typedef {object} ArtifactInfo
+ * @property {string} componentName The library name, e.g. "lib.x"
+ * @property {Set<string>} bundledComponents The embedded components which have a embeddedBy reference to the library


Field embeds in created manifest.json file now contains all nested manifests, independent from the reverse field embeddedBy (embeddedBy points from an embedded manifest to the library's manifest). Remove unreachable code (in createSapUI5() -> getUI5Version() function) . Use path.dirname method to get directory name in findEmbeddedComponents() function. Use string for version value in manifest (in sectionVersion() function). e.g. "1.2.3" instead of the semver object's string representation. It reverts changes which were introduced with #555 Successor of #554

codeworrior requested changes Dec 2, 2020

View reviewed changes

tobiasso85 force-pushed the versioninfo-enrich-with-manifest-infos branch from 8c26c7a to d2e32e8 Compare December 2, 2020 13:52

tobiasso85 force-pushed the versioninfo-enrich-with-manifest-infos branch from 6fb5c10 to 61cfd7c Compare December 23, 2020 11:23

tobiasso85 force-pushed the versioninfo-enrich-with-manifest-infos branch from e346f0c to e2cb917 Compare January 4, 2021 14:20

tobiasso85 changed the title ~~[INTERNAL][WIP] Versioninfo enrich with manifest infos~~ [INTERNAL] Versioninfo enrich with manifest infos Jan 7, 2021

tobiasso85 requested review from codeworrior, matz3 and RandomByte January 8, 2021 16:11

tobiasso85 changed the title ~~[INTERNAL] Versioninfo enrich with manifest infos~~ [FEATURE] Versioninfo enrich with manifest infos Jan 13, 2021

codeworrior reviewed Jan 14, 2021

View reviewed changes

tobiasso85 added 19 commits January 14, 2021 17:08

[INTERNAL][WIP] VersionInfo: with manifest infos

bb1aa26

[INTERNAL][WIP] VersionInfo: with manifest infos

93424fc

improve code by passing correct arguments

[INTERNAL][WIP] VersionInfo: with manifest infos

32b1d3e

adjusted logic to resolve dependencies

[INTERNAL][WIP] VersionInfo: with manifest infos

da9b12c

adjusted logic to resolve dependencies correctly with regards to lazy

[INTERNAL][WIP] VersionInfo: with manifest infos

58fea7d

added logic to use info for embeds

[INTERNAL][WIP] VersionInfo: with manifest infos

475ef28

improved components structure

[INTERNAL][WIP] VersionInfo: with manifest infos

3b20dbd

minor adjustments

[INTERNAL][WIP] VersionInfo: with manifest infos

a445bee

minor adjustments

[INTERNAL][WIP] VersionInfo: with manifest infos

f4ea6b7

simplified merge logic

[INTERNAL][WIP] VersionInfo: with manifest infos

6d6af3b

improved test sort keys of objects

[INTERNAL][WIP] VersionInfo: with manifest infos

b89fff2

adjusted jsdoc of libraryInfos

[INTERNAL][WIP] VersionInfo: with manifest infos

e2b8c84

added TODOs

[FEATURE] manifestCreator: i18n section v22

058479f

refactoring of dependency resolution logic

[FEATURE] manifestCreator: i18n section v22

765bc97

fix recursive logic such that lazy property correctly gets interpreted: * dependencies of lazy dependency are also lazy * having the same dependency lazy and non-lazy results in a non-lazy dependency

[FEATURE] manifestCreator: i18n section v22

f41a621

Rename types to avoid confusion

[FEATURE] manifestCreator: i18n section v22

fa54efc

Rename variable to componentName because it is not the library name

give the code a better structure by using explicit classes

9748592

add tests for embeddedBy to increase test coverage

e885412

tobiasso85 added 8 commits January 14, 2021 18:05

Add test to ensure versionInfoGenerator remains backward compatible

2526ebd

Leaving out optional parameters like libraryManifest and manifestResources must produce the same result as before

fix jsdoc example

1fe9c25

use typedef instead of own class for a data container do embedded component dependencies resolving after library dependencies resolving in separate step to avoid mixing them up.

fix sample in jsdoc

ea58d08

use tag <pre> in jsdoc to preserve formatting Use the same sample across the code e.g. lib.x.sub

manifestResources parameter contains only embedded

874b160

manifests and not the library manifest anymore in the tests adjust expected version info content the same order of properties as the json string result would contain for better readability.

when comparing the version info json created

57acbcb

compare objects and omit the exact buildTimestamp

add more tests document parent lazy flag better

a7e1623

improve error message when dependency was found in manifest

f14cba5

but not in project dependencies improve recursive resolve logic improve jsdoc for ArtifactInfo use better structures instead of classes for * libs and libsResolved * DependencyInfoObject Simplify tests

remove typedef for DependencyInfoObject

666b3cb

refactor duplicate code to common method processManifestAndGetArtifactInfo

tobiasso85 requested a review from codeworrior January 18, 2021 16:53

improve jsdoc and reuse types

b9da72d

add tests for versionInfoGenerator with new parameters * libraryManifest * embeddedManifests

codeworrior reviewed Jan 21, 2021

View reviewed changes

improve code

fd6a8e9

use typedef for ArtifactInfo instead of class because functionality could be extracted and it served then only as a data container. replaced Object.keys().forEach loops which Object.entries loop Use getResolvedLibraries for directly resolving transitive dependencies

tobiasso85 requested a review from codeworrior January 22, 2021 13:54

codeworrior reviewed Jan 26, 2021

View reviewed changes

secure looping by checking libs existence in manifest

8eeab50

simplify manifest path resolving

tobiasso85 requested a review from codeworrior January 26, 2021 10:23

codeworrior previously approved these changes Jan 26, 2021

View reviewed changes

RandomByte requested changes Jan 26, 2021

View reviewed changes

RandomByte reviewed Jan 26, 2021

View reviewed changes

Change logging for missing libraryManifest to verbose

42ce003

With regards to backward compatibility the libraryManifest can be missing. When --all is omitted the libraryManifest can also be missing.

tobiasso85 dismissed codeworrior’s stale review via 42ce003 January 26, 2021 17:31

make LibraryInfo typedef public

daf5610

It is used in the versionInfoGenerator's public JSDoc.

RandomByte approved these changes Jan 26, 2021

View reviewed changes

tobiasso85 merged commit 7603ece into master Jan 26, 2021

tobiasso85 deleted the versioninfo-enrich-with-manifest-infos branch January 26, 2021 17:54

RandomByte requested a review from KlattG January 26, 2021 18:10

KlattG reviewed Jan 28, 2021

View reviewed changes

tobiasso85 mentioned this pull request Feb 16, 2021

[FIX] manifestCreator: 'embeds' should list all components #575

Merged

matz3 mentioned this pull request Mar 4, 2021

[FIX] versionInfoGenerator: fix hasOwnPreload flag #591

Merged



		// process library infos
		const librariesPromises = options.libraryInfos.map((libraryInfo) => {

		@@ -14,6 +16,335 @@ function getTimestamp() {
		return year + month + day + hours + minutes;

[FEATURE] Versioninfo enrich with manifest infos #554

[FEATURE] Versioninfo enrich with manifest infos #554

Conversation

tobiasso85 commented Nov 27, 2020 • edited Loading

manifestHints

components

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeworrior Jan 14, 2021 • edited by RandomByte Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeworrior left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasso85 Jan 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasso85 Jan 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasso85 Jan 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeworrior left a comment

Choose a reason for hiding this comment

RandomByte Jan 26, 2021 • edited Loading

tobiasso85 commented Nov 27, 2020 •

edited

Loading

coveralls commented Jan 4, 2021 •

edited

Loading

codeworrior Jan 14, 2021 •

edited by RandomByte

Loading

tobiasso85 Jan 22, 2021 •

edited

Loading

tobiasso85 Jan 22, 2021 •

edited

Loading

tobiasso85 Jan 22, 2021 •

edited

Loading

RandomByte Jan 26, 2021 •

edited

Loading