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

automation: prep for using new bucket #7299

Merged
merged 4 commits into from Feb 4, 2024

Conversation

chreekat
Copy link
Member

@chreekat chreekat commented Jan 30, 2024

The main substantive change is adding support for AWS_ENDPOINT_URL and setting the name of the bucket for uploading Haddocks (8f8fc12). But I also updated curator to a version that supports the --bucket option and included one change I used while testing the script.

@alaendle
Copy link
Member

lgtm, and seems pretty straight forward.. I could merge and run it; however since I would not consider myself a shell-scripting expert I would ask for another pair of eyes.

juhp

This comment was marked as duplicate.

automated/build.sh Show resolved Hide resolved
@@ -5,6 +5,12 @@ set -eu +x -o pipefail
ROOT=$(cd $(dirname $0) ; pwd)
TARGET=$1

# Home on the container
: ${C_HOME:=$HOME}
Copy link
Contributor

Choose a reason for hiding this comment

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

It has always annoyed me that we have a different path in the container compared to the host.

Though this change seems mostly cosmetic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this just allowed me to override the variable in testing, but doesn't do anything different on its own.

automated/build.sh Show resolved Hide resolved
@juhp juhp merged commit 21ede13 into commercialhaskell:master Feb 4, 2024
1 check passed
alaendle added a commit that referenced this pull request Feb 5, 2024
This reverts commit 21ede13, reversing
changes made to 99acb5a.
@@ -146,7 +151,7 @@ docker run $ARGS_UPLOAD $IMAGE /bin/bash -c "exec curator check-target-available
#
# * Upload the docs to S3
# * Upload the new snapshot .yaml file to the appropriate Github repo, also upload its constraints
docker run $ARGS_UPLOAD $IMAGE /bin/bash -c "curator upload-docs --target $TARGET && curator upload-github --target $TARGET"
docker run $ARGS_UPLOAD $IMAGE /bin/bash -c "curator upload-docs --target $TARGET ${DOCS_BUCKET:+--bucket $DOCS_BUCKET}" && curator upload-github --target $TARGET"
Copy link
Member

Choose a reason for hiding this comment

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

Reverted the PR because in this line the " doesn't match - also I noticed quite strange log messages during running this script mentioning a LTS while I was running a nightly build. Hope this had no unwanted side effects.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, good catch. Of course I modified this line after running the script locally. You can't write a single line of bash without introducing a bug 😄

Copy link
Contributor

@juhp juhp Feb 5, 2024

Choose a reason for hiding this comment

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

Probably good idea to run it through shellcheck also (not saying you have to fix all the shellcheck warnings).
(Though it doesn't catch the undefined NOPLAN.)

chreekat added a commit to chreekat/stackage that referenced this pull request Feb 7, 2024
alaendle pushed a commit that referenced this pull request Feb 7, 2024
* Revert "Revert "Merge pull request #7299 from chreekat/b/new-bucket-prep""

This reverts commit 76a9ef5.

* automation: fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants