You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/05/31 18:03:30 UTC
[GitHub] [iceberg] RussellSpitzer opened a new pull request, #4919: Dev: Release Script Fix
RussellSpitzer opened a new pull request, #4919:
URL: https://github.com/apache/iceberg/pull/4919
Default is wrong on remote
Key values are not used
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886067747
##########
dev/source-release.sh:
##########
@@ -112,7 +112,7 @@ tarball=$tag.tar.gz
git archive $release_hash --worktree-attributes --prefix $tag/ -o $projectdir/$tarball
echo "Signing the tarball..."
-[[ -z "$keyid" ]] && keyopt="-u $keyid"
+[[ -n "$keyid" ]] && keyopt="-u $keyid"
Review Comment:
Nit / follow up - eventually we might want to move away from these admittedly “idiomatic” bash one-liners to something that’s a bit more readable but that’s a style preference for another day.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886065453
##########
dev/source-release.sh:
##########
@@ -36,7 +36,7 @@ usage () {
}
# Default repository remote name
-remote="origin"
+remote="apache"
Review Comment:
Sorry, the link is meant to show this line above
``` echo " -g Specify Git remote name. Defaults to \"apache\""```
We basically state that the default is apache, but it isn't. It is origin.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886203054
##########
dev/source-release.sh:
##########
@@ -112,7 +112,7 @@ tarball=$tag.tar.gz
git archive $release_hash --worktree-attributes --prefix $tag/ -o $projectdir/$tarball
echo "Signing the tarball..."
-[[ -z "$keyid" ]] && keyopt="-u $keyid"
+[[ -n "$keyid" ]] && keyopt="-u $keyid"
Review Comment:
What I've done in the past is add a function that logs either way, so logs X on true and Y on false, so we'd get `No keyid is found in variable \\$keyid, skipping adding signature to tarball` in the untrue path (just some indicator it was taken).
##########
dev/source-release.sh:
##########
@@ -112,7 +112,7 @@ tarball=$tag.tar.gz
git archive $release_hash --worktree-attributes --prefix $tag/ -o $projectdir/$tarball
echo "Signing the tarball..."
-[[ -z "$keyid" ]] && keyopt="-u $keyid"
+[[ -n "$keyid" ]] && keyopt="-u $keyid"
Review Comment:
What I've done in the past is add a function that logs either way, so logs X on true and Y on false, so we'd get `No keyid is found in variable \\$keyid, skipping adding signature to tarball` in the untrue path (just some indicator that the failed path was taken).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886064694
##########
dev/source-release.sh:
##########
@@ -36,7 +36,7 @@ usage () {
}
# Default repository remote name
-remote="origin"
+remote="apache"
Review Comment:
I think this depends on where the script gets run (in the case of origin) and then how the user / box has their git remote set up.
Like mine is `upstream` vs apache, but I know most people / PMC members specifically probably use apache.
EDIT - FYI the link takes me back to this page (at no particular spot in the diff).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886068419
##########
dev/source-release.sh:
##########
@@ -112,7 +112,7 @@ tarball=$tag.tar.gz
git archive $release_hash --worktree-attributes --prefix $tag/ -o $projectdir/$tarball
echo "Signing the tarball..."
-[[ -z "$keyid" ]] && keyopt="-u $keyid"
+[[ -n "$keyid" ]] && keyopt="-u $keyid"
Review Comment:
Yeah it took me a couple takes to realize the mistake here
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886210116
##########
dev/source-release.sh:
##########
@@ -112,7 +112,7 @@ tarball=$tag.tar.gz
git archive $release_hash --worktree-attributes --prefix $tag/ -o $projectdir/$tarball
echo "Signing the tarball..."
-[[ -z "$keyid" ]] && keyopt="-u $keyid"
+[[ -n "$keyid" ]] && keyopt="-u $keyid"
Review Comment:
Yeah trick here is missing that arg still signs, just with the default key
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886006197
##########
dev/source-release.sh:
##########
@@ -36,7 +36,7 @@ usage () {
}
# Default repository remote name
-remote="origin"
+remote="apache"
Review Comment:
https://github.com/apache/iceberg/pull/4919/files#diff-ee187088d9f56516cc2abd9e4b4294a92d23dc09746db5b0976799131c293d2bR33
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886065453
##########
dev/source-release.sh:
##########
@@ -36,7 +36,7 @@ usage () {
}
# Default repository remote name
-remote="origin"
+remote="apache"
Review Comment:
Sorry, the link is meant to show this line above
``` echo " -g Specify Git remote name. Defaults to \"apache\""```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer merged pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
RussellSpitzer merged PR #4919:
URL: https://github.com/apache/iceberg/pull/4919
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886065787
##########
dev/source-release.sh:
##########
@@ -112,7 +112,7 @@ tarball=$tag.tar.gz
git archive $release_hash --worktree-attributes --prefix $tag/ -o $projectdir/$tarball
echo "Signing the tarball..."
-[[ -z "$keyid" ]] && keyopt="-u $keyid"
+[[ -n "$keyid" ]] && keyopt="-u $keyid"
Review Comment:
👍
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886064694
##########
dev/source-release.sh:
##########
@@ -36,7 +36,7 @@ usage () {
}
# Default repository remote name
-remote="origin"
+remote="apache"
Review Comment:
I think this depends on where the script gets run (in the case of origin) and then how the user / box has their git remote set up.
Like mine is `upstream` vs apache, but I know most people / PMC members specifically probably use apache.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886066918
##########
dev/source-release.sh:
##########
@@ -36,7 +36,7 @@ usage () {
}
# Default repository remote name
-remote="origin"
+remote="apache"
Review Comment:
Ahhh! Ok then absolutely yes let’s make it apache. This typically gets run manually in somebody’s machine, so `origin` is very likely incorrect (typically one’s fork).
But if it says the default is `apache`, it should be that.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#issuecomment-1142573875
Also as an aside, during the _actual_ release process, we need to tag the SHA we build off of in master as `release-base-X.Y.0` for a new (non-patch) release.
I don’t think that step is automated anywhere but it’s how the next SNAPSHOT version gets its name (via a function in build.gradle).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a diff in pull request #4919: Dev: Release Script Fix
Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4919:
URL: https://github.com/apache/iceberg/pull/4919#discussion_r886223444
##########
dev/source-release.sh:
##########
@@ -112,7 +112,7 @@ tarball=$tag.tar.gz
git archive $release_hash --worktree-attributes --prefix $tag/ -o $projectdir/$tarball
echo "Signing the tarball..."
-[[ -z "$keyid" ]] && keyopt="-u $keyid"
+[[ -n "$keyid" ]] && keyopt="-u $keyid"
Review Comment:
Oh haha you’re right. This is why I advocate for logging somewhat verbosely in shell scripts.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org