Skip to content

ci(vss): auto-extract Gradle version number ffg upgrade #593

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

Merged
merged 1 commit into from
Jul 10, 2025

Conversation

enigbe
Copy link
Contributor

@enigbe enigbe commented Jul 7, 2025

What this PR does

  • Extract and use Gradle version instead of hard-coded value.

Related Issues

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jul 7, 2025

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot ldk-reviews-bot requested a review from tnull July 7, 2025 20:01
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good generally, some questions.

@@ -61,10 +61,15 @@ jobs:
run: |
# Print Info
java -version
gradle --version
GRADLE_VERSION=$(gradle --version | grep "Gradle" | awk '{print $2}')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we just fail here? If we can't extract the version it probably means something during the install went wrong or something unexpected happened, no? I think we should then just abort here rather than continuing with a default, which might just be confusing to debug if it ever gets hit?

echo "Failed to extract Gradle version, using default"
GRADLE_VERSION="9.0.0-rc-1"
fi
echo $GRADLE_VERSION
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe still print the entire gradle --version in addition to the extracted value?

@@ -61,12 +61,12 @@ jobs:
run: |
# Print Info
java -version
GRADLE_VERSION=$(gradle --version | grep "Gradle" | awk '{print $2}')
GRADLE_VERSION=$(gradle --version | awk '/^Gradle/ {print $2}' | head -1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems some of these changes should go in the first commit?

@enigbe enigbe force-pushed the 2025-07-fix-vss-integration-ci branch from 0cef221 to ef9254a Compare July 9, 2025 23:14
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

LGTM, feel free to squash!

@enigbe enigbe force-pushed the 2025-07-fix-vss-integration-ci branch from ef9254a to 0392313 Compare July 10, 2025 09:49
update to Gradle 9.0 and auto-extract version
number.
@enigbe enigbe force-pushed the 2025-07-fix-vss-integration-ci branch from 0392313 to 8c157c3 Compare July 10, 2025 10:23
@enigbe
Copy link
Contributor Author

enigbe commented Jul 10, 2025

Squashed!

@tnull tnull merged commit 57e1fb1 into lightningdevkit:main Jul 10, 2025
14 of 15 checks passed
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.

VSS CI job is failing
3 participants