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

Build Type mergeBuild #152

Merged
merged 28 commits into from
Nov 18, 2021
Merged

Conversation

dennis-behm
Copy link
Member

This enhancement implements a new build strategy in zAppBuild for topic branches and addresses #151

The purpose of --mergeBuild is to calculate changes which will be merged from the current configuration into the target configuration.

It uses a triple-dot git diff to compare the HEAD to the target remote configuration using the mainBuildBranch.

The changes include

  • Implementation and Documentation of the new build strategy
  • Test case for the zAppBuild Test Framework

As mentioned in the documentation --mergeBuild only builds the changes. It does not perform impact analysis and is not incremental; it always processes the list returned by the git diff. Ex.: When you change one file in your topic branch, mergeBuild will process this file. When you change and commit a second file, then both files will be built.

Copy link
Collaborator

@drbruce-git drbruce-git left a comment

Choose a reason for hiding this comment

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

Looks fine to me with just a reminder of a missing link in the documentation. I am interested in he opinion of @jbyibm

BUILD.md Outdated Show resolved Hide resolved
outgoingChangesBuild

Conflicts:
	test/applications/MortgageApplication/test.properties
Copy link
Member

@jbyibm jbyibm left a comment

Choose a reason for hiding this comment

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

Can you review my comment about full build.

build.groovy Outdated
Comment on lines 432 to 436
if (repositoryClient) {
buildSet = buildUtils.createFullBuildList() }
else {
println "*! Full build requires a repository client connection to a DBB web application"
}
Copy link
Member

Choose a reason for hiding this comment

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

You are changing the current behavior here. Any reason to force providing a repository connection in case of a full build?
I very often perform a full build without repository connection just to test that my whole application can compile.

Copy link
Member Author

@dennis-behm dennis-behm Nov 18, 2021

Choose a reason for hiding this comment

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

Interesting use case, I have not thought about this case so far. Always interesting to see the different usages.

Doesn't your use cause an issue when zAppBuild would like to update the collection? See line 517 in build.groovy

I felt it was a missing test for the fullBuild actually . Happy to revert this.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this is behavior that @drbruce-git has intentionally implemented in its design.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have reverted it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I had always assumed that full build required a repository client so that it can store collections and build results for subsequent incremental builds.

Copy link
Member

@jbyibm jbyibm left a comment

Choose a reason for hiding this comment

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

Look good

@dennis-behm dennis-behm merged commit e3f2b15 into IBM:develop Nov 18, 2021
@dennis-behm dennis-behm deleted the outgoingChangesBuild branch November 18, 2021 14:28
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.

3 participants