You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2020/03/18 19:54:31 UTC

[kudu-CR] [build] Skip builds with no functional changes

Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15481


Change subject: [build] Skip builds with no functional changes
......................................................................

[build] Skip builds with no functional changes

This patch introduces the option to skip builds in pre-commit when
the changes are in files or directories that do not impact the build
or test.

This functionality is disabled by default but can be enabled on our Jenkins instance by setting `KUDU_ALLOW_SKIPPED_TESTS=1`.

I also hid the `DONT_BUILD` functionality behind the
`KUDU_ALLOW_SKIPPED_TESTS` flag to ensure commits with
`DONT_BUILD` in them do not prevent other non-precomit builds
that are using this script from running.

Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
---
M build-support/jenkins/build-and-test.sh
1 file changed, 16 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15481/1
-- 
To view, visit http://gerrit.cloudera.org:8080/15481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
Gerrit-Change-Number: 15481
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>

[kudu-CR] [build] Skip builds with no functional changes

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15481

to look at the new patch set (#5).

Change subject: [build] Skip builds with no functional changes
......................................................................

[build] Skip builds with no functional changes

This patch introduces the option to skip builds in pre-commit when
the changes are in files or directories that do not impact the build
or test.

This functionality is disabled by default but can be enabled on our
Jenkins instance by setting `KUDU_ALLOW_SKIPPED_TESTS=1`.

I also hid the `DONT_BUILD` functionality behind the
`KUDU_ALLOW_SKIPPED_TESTS` flag to ensure commits with
`DONT_BUILD` in them do not prevent other builds that are using
this script from running.

Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
---
M build-support/jenkins/build-and-test.sh
1 file changed, 25 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15481/5
-- 
To view, visit http://gerrit.cloudera.org:8080/15481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
Gerrit-Change-Number: 15481
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [build] Skip builds with no functional changes

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15481 )

Change subject: [build] Skip builds with no functional changes
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/15481/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15481/4//COMMIT_MSG@9
PS4, Line 9: This patch introduces the option to skip builds in pre-commit when
           : the changes are in files or directories that do not impact the build
           : or test.
> I appreciate the motivation behind this, but are you sure this is desirable
I think the safest bet is to ignore things that are "safe" to ignore. Essentially target no false positives, but accept some false negatives.


http://gerrit.cloudera.org:8080/#/c/15481/4//COMMIT_MSG@13
PS4, Line 13: This functionality is disabled by default but can be enabled on our Jenkins instance by setting `KUDU_ALLOW_SKIPPED_TESTS=1`.
> Too long.
Done


http://gerrit.cloudera.org:8080/#/c/15481/4//COMMIT_MSG@17
PS4, Line 17: precomit
> pre-commit
Done


http://gerrit.cloudera.org:8080/#/c/15481/4/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

http://gerrit.cloudera.org:8080/#/c/15481/4/build-support/jenkins/build-and-test.sh@96
PS4, Line 96: .dockerignore
> Could add .gitignore and .ycm_extra_conf.py too.
Done


http://gerrit.cloudera.org:8080/#/c/15481/4/build-support/jenkins/build-and-test.sh@96
PS4, Line 96: examples
> We build the C++ example in client-examples.sh, so we can't exclude the ent
I will drop this directory then in case we choose to build other examples in the future as well.


http://gerrit.cloudera.org:8080/#/c/15481/4/build-support/jenkins/build-and-test.sh@96
PS4, Line 96:      grep -qvE "^\.dockerignore|^docker/|^docs/|^examples/|^kubernetes/|LICENSE\.txt|NOTICE\.txt|.*\.adoc|.*\.md"; then
> Any way we could set this up with each entry on its own line? It'll be much
I am not sure how to wrap this. FWIW we shouldn't be adjusting this too much.



-- 
To view, visit http://gerrit.cloudera.org:8080/15481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
Gerrit-Change-Number: 15481
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 18 Mar 2020 20:56:20 +0000
Gerrit-HasComments: Yes

[kudu-CR] [build] Skip builds with no functional changes

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15481 )

Change subject: [build] Skip builds with no functional changes
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/15481/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15481/4//COMMIT_MSG@9
PS4, Line 9: This patch introduces the option to skip builds in pre-commit when
           : the changes are in files or directories that do not impact the build
           : or test.
I appreciate the motivation behind this, but are you sure this is desirable long term? It can actually be somewhat difficult to determine whether a given file/directory affects the build or a test (see my comment about 'examples'), and it's also going to be annoying to keep this list up-to-date with the reality in the repo.


http://gerrit.cloudera.org:8080/#/c/15481/4//COMMIT_MSG@13
PS4, Line 13: This functionality is disabled by default but can be enabled on our Jenkins instance by setting `KUDU_ALLOW_SKIPPED_TESTS=1`.
Too long.


http://gerrit.cloudera.org:8080/#/c/15481/4//COMMIT_MSG@17
PS4, Line 17: precomit
pre-commit


http://gerrit.cloudera.org:8080/#/c/15481/4/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

http://gerrit.cloudera.org:8080/#/c/15481/4/build-support/jenkins/build-and-test.sh@96
PS4, Line 96: examples
We build the C++ example in client-examples.sh, so we can't exclude the entire directory.


http://gerrit.cloudera.org:8080/#/c/15481/4/build-support/jenkins/build-and-test.sh@96
PS4, Line 96:      grep -qvE "^\.dockerignore|^docker/|^docs/|^examples/|^kubernetes/|LICENSE\.txt|NOTICE\.txt|.*\.adoc|.*\.md"; then
Any way we could set this up with each entry on its own line? It'll be much easier to maintain that way.


http://gerrit.cloudera.org:8080/#/c/15481/4/build-support/jenkins/build-and-test.sh@96
PS4, Line 96: .dockerignore
Could add .gitignore and .ycm_extra_conf.py too.



-- 
To view, visit http://gerrit.cloudera.org:8080/15481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
Gerrit-Change-Number: 15481
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 18 Mar 2020 20:32:38 +0000
Gerrit-HasComments: Yes

[kudu-CR] [build] Skip builds with no functional changes

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15481 )

Change subject: [build] Skip builds with no functional changes
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/15481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
Gerrit-Change-Number: 15481
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 18 Mar 2020 21:25:07 +0000
Gerrit-HasComments: No

[kudu-CR] [build] Skip builds with no functional changes

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15481 )

Change subject: [build] Skip builds with no functional changes
......................................................................

[build] Skip builds with no functional changes

This patch introduces the option to skip builds in pre-commit when
the changes are in files or directories that do not impact the build
or test.

This functionality is disabled by default but can be enabled on our
Jenkins instance by setting `KUDU_ALLOW_SKIPPED_TESTS=1`.

I also hid the `DONT_BUILD` functionality behind the
`KUDU_ALLOW_SKIPPED_TESTS` flag to ensure commits with
`DONT_BUILD` in them do not prevent other builds that are using
this script from running.

Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
Reviewed-on: http://gerrit.cloudera.org:8080/15481
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M build-support/jenkins/build-and-test.sh
1 file changed, 25 insertions(+), 4 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Adar Dembo: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/15481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
Gerrit-Change-Number: 15481
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [build] Skip builds with no functional changes

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15481

to look at the new patch set (#3).

Change subject: [build] Skip builds with no functional changes
......................................................................

[build] Skip builds with no functional changes

This patch introduces the option to skip builds in pre-commit when
the changes are in files or directories that do not impact the build
or test.

This functionality is disabled by default but can be enabled on our Jenkins instance by setting `KUDU_ALLOW_SKIPPED_TESTS=1`.

I also hid the `DONT_BUILD` functionality behind the
`KUDU_ALLOW_SKIPPED_TESTS` flag to ensure commits with
`DONT_BUILD` in them do not prevent other non-precomit builds
that are using this script from running.

Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
---
M build-support/jenkins/build-and-test.sh
1 file changed, 16 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15481/3
-- 
To view, visit http://gerrit.cloudera.org:8080/15481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
Gerrit-Change-Number: 15481
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [build] Skip builds with no functional changes

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15481

to look at the new patch set (#4).

Change subject: [build] Skip builds with no functional changes
......................................................................

[build] Skip builds with no functional changes

This patch introduces the option to skip builds in pre-commit when
the changes are in files or directories that do not impact the build
or test.

This functionality is disabled by default but can be enabled on our Jenkins instance by setting `KUDU_ALLOW_SKIPPED_TESTS=1`.

I also hid the `DONT_BUILD` functionality behind the
`KUDU_ALLOW_SKIPPED_TESTS` flag to ensure commits with
`DONT_BUILD` in them do not prevent other non-precomit builds
that are using this script from running.

Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
---
M build-support/jenkins/build-and-test.sh
1 file changed, 24 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/15481/4
-- 
To view, visit http://gerrit.cloudera.org:8080/15481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If56db6593ce7aa64b562a9fb277a337bffc7b532
Gerrit-Change-Number: 15481
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)