You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by "clayburn (via GitHub)" <gi...@apache.org> on 2023/04/13 13:53:03 UTC
[GitHub] [curator] clayburn commented on a diff in pull request #459: CURATOR-669. Integrate with ge.apache.org Gradle Enterprise server
clayburn commented on code in PR #459:
URL: https://github.com/apache/curator/pull/459#discussion_r1165546553
##########
.mvn/ge-extensions.xml:
##########
Review Comment:
I would highly recommend renaming this to `extensions.xml` and removing the `Configure Gradle Enterprise integration` from `ci.yml`. Consider:
* This makes it much easier for a local developer to publish a local build scan without having to move the `ge-extensions.xml` file, whether to https://ge.apache.org or to https://scans.gradle.com.
* This simplifies your CI workflow, requiring no changes to the your `ci` workflow.
* This gives all builds access to the local build cache.
* While we strongly recommend enabling build scans for all builds, a better way to only publish build scans on CI would utilize [criteria-based publishing](https://docs.gradle.com/enterprise/maven-extension/#publishing_based_on_criteria).
* Applying the extension to all builds is quite safe and well tested, even if the build does not publish a build scan.
##########
.mvn/gradle-enterprise.xml:
##########
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<gradleEnterprise
+ xmlns="https://www.gradle.com/gradle-enterprise-maven" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="https://www.gradle.com/gradle-enterprise-maven https://www.gradle.com/schema/gradle-enterprise-maven.xsd">
+ <server>
+ <url>https://ge.apache.org</url>
+ <allowUntrusted>false</allowUntrusted>
+ </server>
+ <buildScan>
+ <capture>
+ <goalInputFiles>true</goalInputFiles>
+ <buildLogging>true</buildLogging>
+ <testLogging>true</testLogging>
+ </capture>
+ <backgroundBuildScanUpload>#{isFalse(env['GITHUB_ACTIONS'])}</backgroundBuildScanUpload>
+ <publish>ALWAYS</publish>
Review Comment:
I would recommend adding `publishIfAuthenticated>true</publishIfAuthenticated>`. It is a hidden option, but will give unauthenticated users a better user experience, given that not every builder of this open source project will be an authenticated user to https://ge.apache.org.
##########
.mvn/gradle-enterprise-custom-user-data.groovy:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// Add Maven command line arguments
+def mavenCommand = ''
+
+if (System.env.MAVEN_CMD_LINE_ARGS) {
+ mavenCommand = "mvn ${System.env.MAVEN_CMD_LINE_ARGS}".toString()
+ buildScan.value('Maven command line', mavenCommand)
Review Comment:
I would be very careful capturing the entire Maven command line. You could imagine scenarios where a user passes in a secret as a system property, or possibly even typos that include secrets. Gradle Enterprise doesn't capture certain environment details for this reason.
Perhaps it would make more sense to capture specific details you may be interested in? For example, we already add a custom value for the `skipTests` setting. We also have samples to [capture profiles](https://github.com/gradle/gradle-enterprise-build-config-samples/blob/main/build-data-capturing-maven-samples/capture-profiles/maven-profiles.groovy) as tags. This may actually make it easier to queries that combine the different values, rather than trying to construct queries that are considering the command line as a whole. It may also be more accurate, as capturing the value of configured properties would not _only_ capture values passed in at the command line.
##########
.mvn/ge-extensions.xml:
##########
@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+-->
+<extensions xmlns="http://maven.apache.org/EXTENSIONS/1.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/EXTENSIONS/1.0.0 http://maven.apache.org/xsd/core-extensions-1.0.0.xsd">
+ <extension>
+ <groupId>com.gradle</groupId>
+ <artifactId>gradle-enterprise-maven-extension</artifactId>
+ <version>1.16.3</version>
Review Comment:
The most recent version of this extension is 1.16.6. Once https://ge.apache.org is update to 2023.1, you may update this to 1.17
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org