-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
GH-3091: Add verification guide and .rat-excludes.txt for release #3101
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
.gitignore | ||
.rat-excludes.txt | ||
PULL_REQUEST_TEMPLATE.md | ||
strings-2.parquet$ | ||
nested_array.avsc$ | ||
map_with_nulls.avsc$ | ||
map.avsc$ | ||
list_with_nulls.avsc$ | ||
fixedToInt96.avsc$ | ||
array.avsc$ | ||
allFromParquetOldBehavior.avsc$ | ||
allFromParquetNewBehavior.avsc$ | ||
all.avsc$ | ||
stringBehavior.avsc$ | ||
logicalType.avsc$ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -498,6 +498,7 @@ | |
<consoleOutput>true</consoleOutput> | ||
<excludes> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am pretty sure we can change |
||
<exclude>.github/PULL_REQUEST_TEMPLATE.md</exclude> | ||
<exclude>.rat-excludes.txt</exclude> | ||
<exclude>**/*.parquet</exclude> | ||
<exclude>**/*.avro</exclude> | ||
<exclude>**/*.json</exclude> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this! Is it better to add this to the parquet site: https://github.com/apache/parquet-site/blob/production/content/en/docs/Contribution%20Guidelines/releasing.md?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree the "release verification" should be moved to the
parquet-site
repo instead. Then, we even can have a link to this section in the VOTE email template.I'm not sure why we need to check for the license headers separately in the tarball. It is already in the build process so we shall not have license header issues in the repo. What I usually do instead is comparing the content of the tarball with a freshly cloned repo set to the release RC tag. There should be no differences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, we can probably remove explicitly checking license headers. This is something that in general I've seen all projects do as part of their release verification process and something that I would say falls under the "verify that they meet all requirements of ASF policy on releases as described below" point on the ASF release guide. But it is true that as soon as there hasn't been any change as those are already done feels unnecessary.
How do you perform the comparison between the content of the tarball with a freshly cloned repo set to the release RC tag? Do we want to add that as a step?
I will move the PR to the parquet-site one, I might take a couple of days as I am slightly busy at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use meld as diff tool but I don't think it should be added. Probably GNU
diff
can be configured to work on directory trees.I don't think we need to hurry. Please refer the
parquet-site
PR here so anyone call follow up.Thanks a lot for working on this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the
parquet-site
is more suitable for these steps.Regarding the license headers. It is part of the verification; having the
rat
check is just one way. All code must have an ASv2 license header. It would also be good to do manual checks when a new version is being released, as the RAT check might also miss something.Thanks for working on this, this is really great 🙌