added script and doc to automate release in the future#33
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a documentation guide and an automation script (sync-upstream.sh) to streamline syncing the repository with the upstream BoringSSL repository while preserving local SwiftPM configurations. The reviewer provided several constructive suggestions to improve the robustness of the script, including adding missing directories like docs/ and .github/ to the preservation list, fixing a potential nested directory bug during restoration, utilizing a trap for reliable temporary directory cleanup, verifying the existence of the upstream remote, and excluding the .git/ directory from cleanup search paths. Additionally, formatting improvements were suggested for the documentation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
@cherylEnkidu @ncooke3 Please review. |
ncooke3
left a comment
ncooke3
left a comment
There was a problem hiding this comment.
Nice! LGTM with a couple nits and questions
cherylEnkidu
left a comment
cherylEnkidu
left a comment
There was a problem hiding this comment.
Thank you for writing up this script!
Additionally, we need to ensure that no test code remains in the repository, aligning with the architectural rule we established in our previous review (that test code should not be built or linked into the library at all). To support this, please remove swiftpmtests/ from the PRESERVE list since we have already deleted all local test targets. Furthermore, please update the cleanup find command to delete C test files (*_test.c like ssl_c_test.c) rather than just C++ and Go tests; this will automatically purge upstream C tests during the sync and prevent us from having to manually exclude them in the package manifest in the future.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Nick Cooke <36927374+ncooke3@users.noreply.github.com>
@cherylEnkidu if we remove swiftpmtest from preserve list, build will be failed. |
|
@ncooke3 @cherylEnkidu suggested changes have been addressed. |
| rm -rf .swiftpm/ .bcr/ .build/ | ||
|
|
||
| # Files and artifacts | ||
| find . -type f -not -path "./.git/*" \( -name "*_test.cc" -o -name "*_test.go" -o -name "*_tests.txt" -o -name "*_unittest.cc" \) -delete |
There was a problem hiding this comment.
find . -type f -not -path "./.git/*" \( -name "*_test.cc" -o -name "*_test.go" -o -name "*_tests.txt" -o -name "*_unittest.cc" -o -name "*_test.c" \) -delete
I think we can include .c test files in the cleanup pattern
| @@ -0,0 +1,79 @@ | |||
| #!/bin/bash | |||
| set -e | |||
|
|
|||
There was a problem hiding this comment.
We can add a working directory anchor at the top of scripts/sync-upstream.sh (right under set -e) so it runs correctly even if invoked from inside the scripts/ subdirectory:
# Move to repository root
cd "$(dirname "$0")/.."
3 participants
Script added to sync from the google/boringssl so in future it wont take much time to release new version.