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

MINOR: Add license header in suppressions.xml #11753

Merged
merged 1 commit into from Feb 17, 2022

Conversation

ruanwenjun
Copy link
Member

I find there is no Apache License header in suppressions.xml, this pr is adding the license header.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Copy link
Contributor

@showuon showuon left a comment

Choose a reason for hiding this comment

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

@ruanwenjun , thanks for the patch. Left a comment.

@@ -1,9 +1,24 @@

<?xml version="1.0"?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary to add this line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your review.
To be honestly, I am not sure if this is must, but from the demo provided by checkstyle, it contains this header.
So it might be better to add this header.
https://maven.apache.org/plugins/maven-checkstyle-plugin/examples/suppressions-filter.html

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, from the XML spec [1], it said:

XML documents SHOULD begin with an XML declaration which specifies the version of XML being used.

Adding it to follow the spec suggestion is good to me.
1: https://www.w3.org/TR/xml/#sec-prolog-dtd

@showuon
Copy link
Contributor

showuon commented Feb 17, 2022

failed tests are unrelated:

[Build / JDK 11 and Scala 2.13 / kafka.server.DynamicBrokerReconfigurationTest.testThreadPoolResize()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-11753/1/testReport/junit/kafka.server/DynamicBrokerReconfigurationTest/Build___JDK_11_and_Scala_2_13___testThreadPoolResize__/)
    [Build / JDK 11 and Scala 2.13 / kafka.server.DynamicBrokerReconfigurationTest.testThreadPoolResize()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-11753/1/testReport/junit/kafka.server/DynamicBrokerReconfigurationTest/Build___JDK_11_and_Scala_2_13___testThreadPoolResize___2/)
    Build / JDK 8 and Scala 2.12 / org.apache.kafka.connect.integration.ConnectorRestartApiIntegrationTest.testMultiWorkerRestartOnlyConnector
    [Build / JDK 8 and Scala 2.12 / kafka.api.ConsumerBounceTest.testClose()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-11753/1/testReport/junit/kafka.api/ConsumerBounceTest/Build___JDK_8_and_Scala_2_12___testClose__/)
    [Build / JDK 17 and Scala 2.13 / kafka.server.DynamicBrokerReconfigurationTest.testThreadPoolResize()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-11753/1/testReport/junit/kafka.server/DynamicBrokerReconfigurationTest/Build___JDK_17_and_Scala_2_13___testThreadPoolResize__/)
    [Build / JDK 17 and Scala 2.13 / kafka.server.DynamicBrokerReconfigurationTest.testThreadPoolResize()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-11753/1/testReport/junit/kafka.server/DynamicBrokerReconfigurationTest/Build___JDK_17_and_Scala_2_13___testThreadPoolResize___2/)

Copy link
Contributor

@showuon showuon left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the patch, and help format the license header in xml files.

@showuon showuon merged commit 760e6f3 into apache:trunk Feb 17, 2022
@ruanwenjun ruanwenjun deleted the dev_wenjun_addLicenseHeader branch March 16, 2022 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants