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

Update build.groovy #515

Closed
wants to merge 1 commit into from
Closed

Update build.groovy #515

wants to merge 1 commit into from

Conversation

GeraldMit
Copy link
Member

finalizeBuildProcess. optional second param error path issue

NPE avoidance for buildOrder (caught instead at required Properties)

finalizeBuildProcess. optional second param error path issue

NPE avoidance for buildOrder (caught instead at required Properties)
Copy link
Member

@dennis-behm dennis-behm left a comment

Choose a reason for hiding this comment

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

@GeraldMit

There are additional problems being introduced with these changes.

The finalise Method is accessing count from the args map:

https://github.com/IBM/dbb-zappbuild/pull/515/files#diff-665cfd7c0dbe02e87a7dcb77c326a4e1a22b1fdd7e400977d6e6e49d77602a70R815

This should actually be tested if count is in the map.

	if(args.count) {println("** Total files processed : ${args.count}") }

In this section - we could then just pass the startTime, with the above update.
https://github.com/IBM/dbb-zappbuild/pull/515/files#diff-665cfd7c0dbe02e87a7dcb77c326a4e1a22b1fdd7e400977d6e6e49d77602a70R42

The buildOrder should actually be tested as a required build property:

#
# Comma separated list of required build properties for zAppBuild/build.groovy
requiredBuildProperties=buildOrder,buildListFileExt
+

dbb-zappbuild/build.groovy

Lines 126 to 127 in a719842

// verify required build properties
buildUtils.assertBuildProperties(props.requiredBuildProperties)

So, I believe your problem has a different root cause.

@@ -711,6 +711,9 @@ def createBuildList() {
}

def finalizeBuildProcess(Map args) {
finalizeBuildProcess(args, 0)
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest we follow the same strategy and access the attributes in the map instead of overriding the finalizeBuildProcess method

@dennis-behm
Copy link
Member

This PR will be superseded by - #522

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