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

feat(dev): add command to update composer deps #6773

Merged
merged 10 commits into from
Nov 21, 2023
Merged

Conversation

bshaffer
Copy link
Contributor

@bshaffer bshaffer commented Nov 3, 2023

Command to set or bump composer deps.

This will be useful for implementing a GitHub Action to run the google/cloud package tests against new versions of GAX and Auth libraries before release.

TODO

  • add tests for the new command
  • consider using this command to replace logic done in: (this became overly complicated)
    for i in BigQuery,cloud-bigquery Core,cloud-core Logging,cloud-logging PubSub,cloud-pubsub Storage,cloud-storage ShoppingCommonProtos,shopping-common-protos,0.1; do
    IFS=","; set -- $i;
    if grep -q "\"google/$2\":" ${DIR}/composer.json; then
    if [ -z "$3" ]; then VERSION="1.100"; else VERSION=$3; fi
    composer config repositories.$2 "{\"type\": \"path\", \"url\": \"../$1\", \"options\":{\"versions\":{\"google/$2\":\"$VERSION\"}}}" -d ${DIR}
    fi
    done

@bshaffer bshaffer marked this pull request as ready for review November 9, 2023 22:27
@bshaffer bshaffer requested review from a team as code owners November 9, 2023 22:27
@bshaffer bshaffer added the next release PRs to be included in the next release label Nov 10, 2023
if [ $? != 0 ]; then
echo "$DIR: composer install failed" >> "${FAILED_FILE}"
# run again but without "-q" so we can see the error
COMPOSER_ROOT_VERSION=$(cat $DIR/VERSION) composer --no-interaction --no-ansi --no-progress update -d ${DIR};
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

unrelated to this PR, but should this not be exit instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, because we want to run all the tests even if one fails, so we can see if the others succeed or not. By sending the error in FAILED_FILE, we ensure that we will exit below.

Copy link
Contributor

@vishwarajanand vishwarajanand left a comment

Choose a reason for hiding this comment

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

Lgtm, left a few nits

@bshaffer bshaffer merged commit 8c36f3c into main Nov 21, 2023
23 checks passed
@bshaffer bshaffer deleted the add-update-deps-command branch November 21, 2023 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release PRs to be included in the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants