-
Notifications
You must be signed in to change notification settings - Fork 123
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
Add automatic signing and uploading releases #414
base: master
Are you sure you want to change the base?
Conversation
fa8f4a7
to
55effa8
Compare
There are some things left to fix:
|
The instruction in the just added release instructions can be simplified. Can you include the changes in this PR. Or should I do it and add it to the branch? I find it always useful to write instructions before writting the code. |
Commit: $COMMIT_ID | ||
Distribution: $PROJECT_DIST_URL | ||
Signing key: 0x077e8893a6dcc33dd4a4d5b256e73ba9a0b592d0 | ||
Review kit: https://logging.apache.org/logging-parent/release-review-instructions.html |
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.
The reviewer should confirm the uploaded source code is not corrupt and
is identical to the package generated by the Github action.
This is what I have put in the review kit I created
Title: [ANNOUNCE] $PROJECT_NAME \`$PROJECT_VERSION\` released | ||
|
||
${PROJECT_NAME} team is pleased to announce the \`$PROJECT_VERSION\` | ||
release. ${PROJECT_NAME} is a versatile, industrial-strength |
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.
"Apache ${PROJECT_NAME} is a logging framework for C++"
- name: Checkout repository | ||
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # 4.2.1 | ||
with: | ||
persist-credentials: false # do not persist auth token in the local git config |
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.
Add ref: v${{project-version}}-RC${{release-candidate}
Rational: Preparing a release begins with the assignment of a tag. (e.g. I have already assigned v1.3.0-RC1 )
inputs: | ||
project-version: | ||
description: The version of Log4cxx | ||
default: 1.3.0 |
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.
Add
release-candidate:
description: The tag suffix (a number) appended to "v${{project-version}}-RC"
required: true
Rational: Preparing a release begins with the assignment of a tag. (e.g. I have already assigned v1.3.0-RC1 )
|
||
# Timestamp used for the source archive to guarantee reproducible builds in ISO 8601 format. | ||
# | ||
# To generate use: date -u +%Y-%m-%dT%H:%M:%SZ |
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.
The date should be taken from the commit.
exit 1 | ||
fi | ||
|
||
OUTPUT_TIMESTAMP=$(sed -n -e "s/^set(log4cxx_OUTPUT_TIMESTAMP \"\(.*\)\")/\1/p" < src/cmake/projectVersionDetails.cmake) |
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.
The date should be taken from the commit. This script could require a parameter or read it from a file in the build directory.
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.
That would probably be the ideal solution.
In Log4j we had a chicken-and-egg problem, because changing the timestamp for the JAR files requires the modification of the Maven project.build.outputTimestamp
property, which requires a commit, which modifies the timestamp, etc. 😉
We could however solve the problem by overriding the COMMIT_DATE
of the commit that just modifies project.build.outputTimestamp
.
@@ -12,58 +13,180 @@ | |||
# 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. | |||
# | |||
name: Generate release files | |||
|
|||
on: | |||
workflow_dispatch: |
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 that running an action on tag assignment is possible.
We could perform this action GITHUB_REF matches v[0-9].[0-9].[0-9]-RC[0-9]
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.
In Log4j we run the release action on each push to a release/<version>
branch. Would that be an option for Log4cxx?
The advantage of making a branch vs a tag is that we don't stall a release if the main branch receives new commits.
Thank You for the remarks, they made me realize there is still a lot to do before we automatize all Apache Logging Services releases. I will resolve the conversations above once I have actually resolved them in this PR (i.e. don't stall the Apache Log4cxx release for this PR). I opened a brainstorming thread on |
I don't see anything obviously wrong with what has been proposed here, we'll just have to test it out and see that it works. |
The release is currently stalled anyway because logging.staged.apache.org is broken |
Not anymore: https://logging.staged.apache.org/log4cxx/1.3.0/ |
I have commenced the release process |
This adds support for: