Skip gradle check for non code related changes #13085
Conversation
Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
|
Thoughts here - @dblock / @andrross / @reta / @peternied |
| run_gradle_check=false | ||
|
|
||
| for filename in ${ALL_CHANGED_FILES}; do | ||
| # Run gradle check if file ends in [java/gradle/json] |
There was a problem hiding this comment.
I like the spirit here but there are many files that can fail the build that don't meet these extensions:
- no extension, e.g. https://github.com/opensearch-project/OpenSearch/blob/main/libs/core/src/main/resources/META-INF/services/org.opensearch.core.compress.spi.CompressorProvider
- License files (.txt I think?) are checked as a part of the build
- gradle.properties
- probably many many others?
There was a problem hiding this comment.
We can do an inverse set where we skip gradle check if the changes are limited to certain file types
There was a problem hiding this comment.
If going this route, I'd rather do a strictly positive list. If the commits only affects: [CHANGELOG*.md, README.md, ...] then skip
There was a problem hiding this comment.
👍 to @peternied 's suggestion, other examples are: version.properties, bwcVersions, ....
There was a problem hiding this comment.
That sounds good. I was just bouncing the idea across.
I'll work on refining and testing this out.
Thanks folks.
There was a problem hiding this comment.
I find paths-ignore as an easier option. We can keep on adding to the list and also manage file-types, directories, etc. This won't even initiate the workflow run for ignored paths. Example: #13477
There was a problem hiding this comment.
That sounds good. I'll close out this PR and we can keep adding to paths-ignore
Compatibility status:Checks if related components are compatible with change fae27de Incompatible componentsSkipped componentsCompatible componentsCompatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/performance-analyzer.git] |
peternied
changed the title
|
❌ Gradle check result for fae27de: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
How about something similar to skip-changelog where a maintainer can make a call that a gradle check is not required? |
I mean, we can already do this by just merging the PR without a completed or passing check. Seems a bit like a slippery slope if we normalize manually skipping the gradle check. |
+1 to this. I'd be wary of adding a super power, and instead rely on the framework to guide through it. |
Description
gradle checkin this scenario since the code has not been modified in any wayPros: Getting to merge in non code related changes without any hassles
Cons(?): Missing out on flakiness
I would love to hear some feedback from other maintainers.
Check List
New functionality includes testing.All tests passNew functionality has been documented.New functionality has javadoc addedFailing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)Commit changes are listed out in CHANGELOG.md file (See: Changelog)Public documentation issue/PR createdBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.