visit-dav / visit

VisIt - Visualization and Data Analysis for Mesh-based Scientific Data
https://visit.llnl.gov
BSD 3-Clause "New" or "Revised" License
440 stars 116 forks source link

Adjust Azure CI with branch regex and path filter #19986

Closed markcmiller86 closed 3 weeks ago

markcmiller86 commented 3 weeks ago
markcmiller86 commented 3 weeks ago

@cyrush whadya think about adding logic like this too...

pr:
  branches:
.
.
.
  paths:
    include:
    - '**/CMakeLists.txt'
    - '**/*.cmake'
    - '**/*.in'
    - '**/*.C'
    - '**/*.h'

This way, we only do builds and wait for them to complete when we're changing compilation related stuff.

markcmiller86 commented 3 weeks ago

@biagas regarding the path trigger logic, can you think of anythings other than CMakeLists.txt files and *.[Ch] files that could change to trigger re-compilation?

biagas commented 3 weeks ago

Changes to any of the src/CMake/.cmake files. Possibly also .in (those are usually used with configure_file).

cyrush commented 3 weeks ago

@cyrush whadya think about adding logic like this too...

pr:
  branches:
.
.
.
  paths:
    include:
    - '**/CMakeLists.txt'
    - '**/*.cmake'
    - '**/*.in'
    - '**/*.C'
    - '**/*.h'

This way, we only do builds and wait for them to complete when we're changing compilation related stuff.

I think I am confused. This would only test if we change parts of the build system proper?

I don't think that works. For example, If we change the contents the Silo database reader, we want to make sure we compile and test those changes?

markcmiller86 commented 3 weeks ago

I think I am confused. This would only test if we change parts of the build system proper?

Well, build system plus all source code (.C and .h files)

markcmiller86 commented 3 weeks ago

The ** designation means anywhere in the source code tree.

markcmiller86 commented 3 weeks ago

Lemme just try to go ahead and test that in this PR.

markcmiller86 commented 3 weeks ago

Ok, well, once I changed one .h file in this PR, forevermore it sees this PR as involving "source code" and will execute the CI. But, I did try a .rst file and a .md file change and it skipped the CI for this files, which is better behavior I think. The RTD CI still runs because it is triggered differently (from RTD's end and any changes to files at or below src/doc).

markcmiller86 commented 3 weeks ago

@visit-dav/visit-developers ok what I've done here is added a path filter to the Azure CI stuff so that it will skip it (quickly) if the PR does not involve any files that would effect that CI.

Lemme know what you think.

biagas commented 3 weeks ago

@markcmiller86 seems good to me

markcmiller86 commented 3 weeks ago

Ok, I've tested this a few different ways and it seems to have the intended effects.

markcmiller86 commented 3 weeks ago

@JustinPrivitera and @brugger1 have a look at the logic here now controlling which .py files trigger the CI. Its a tad more involved than I'd like because its not possible to use the value of a variable to indicate the entries in the paths trigger.

JustinPrivitera commented 3 weeks ago

You manually specify the ones that you want to trigger the CI?

markcmiller86 commented 3 weeks ago

You manually specify the ones that you want to trigger the CI?

Yes, with the include: sub-item. There is also an exclude: clause where you can specify stuff that should not be included. That seems harder for this case though.

markcmiller86 commented 3 weeks ago

Ok, so a2653176d74082de6adbafe4357b0ab1f6dac2d7 passed all tests

cyrush commented 3 weeks ago

@markcmiller86 sorry I was confused b/c I misread, yes this is a great idea -- thanks!