Skip to content

Conversation

@ansgarm
Copy link
Member

@ansgarm ansgarm commented Mar 19, 2025

Updated the file as per the CONTRIBUTING.md guide (at the very bottom)

Adds support for pre-releases to Github Action

@ansgarm ansgarm force-pushed the update-plugin-go branch 2 times, most recently from c638524 to 94ce60b Compare March 19, 2025 14:49
Base automatically changed from update-plugin-go to main March 19, 2025 14:51
meta/meta.go Outdated
// Deprecated: Use Go standard library [runtime/debug] package build information
// instead.
var SDKVersion = "2.36.1"
var SDKVersion = "2.37.0"
Copy link
Member

Choose a reason for hiding this comment

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

I believe our CI should do this for us? Not sure about the pre-release portion: 9a1ad1d

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, neat, I did not know!

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it looks like it probably won't update pre-release 😆 , not sure exactly who is using this and where, might not be a big deal ATM

meta-version:
needs: changelog-version
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
# Default input is the SHA that initially triggered the workflow. As we created a new commit in the previous job,
# to ensure we get the latest commit we use the ref for checkout: 'refs/heads/<branch_name>'
ref: ${{ github.ref }}
# Avoid persisting GITHUB_TOKEN credentials as they take priority over our service account PAT for `git push` operations
# More details: https://github.com/actions/checkout/blob/b4626ce19ce1106186ddf9bb20e706842f11a7c3/adrs/0153-checkout-v2.md#persist-credentials
persist-credentials: false
- name: Update meta package SDKVersion
run: sed -i "s/var SDKVersion =.*/var SDKVersion = \"${{ needs.changelog-version.outputs.version }}\"/" meta/meta.go
- name: Git push meta
run: |
git config --global user.name "${{ vars.TF_DEVEX_CI_COMMIT_AUTHOR }}"
git config --global user.email "${{ vars.TF_DEVEX_CI_COMMIT_EMAIL }}"
git add meta/meta.go
git commit -m "Update meta package SDKVersion"
git push "https://${{ vars.TF_DEVEX_CI_COMMIT_AUTHOR }}:${{ secrets.TF_DEVEX_COMMIT_GITHUB_TOKEN }}@github.com/${{ github.repository }}.git"

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll update the contributing docs then once we know what'll happen with the pre-release

Copy link
Member Author

Choose a reason for hiding this comment

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

This line actually might be a problem:

run: sed -i "s/var SDKVersion =.*/var SDKVersion = \"${{ needs.changelog-version.outputs.version }}\"/" meta/meta.go

Copy link
Member Author

Choose a reason for hiding this comment

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

not entirely sure what'll end up in changelog-version.outputs.version but I highly suspect the pre-release identifier might be part of it 🤨

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh damn, Github didn't show your comment 😂 So it now seems like I ignored what you said..

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's one data point -- a search across official provider repos. Two providers use this in a user-agent string, which seems fine.

$ ag meta\.SDKVersion $(ag -l SDKVersion | grep -v vendor)
terraform-provider-azuread/internal/common/client_options.go
109:	tfUserAgent := fmt.Sprintf("HashiCorp Terraform/%s (+https://www.terraform.io) Terraform Plugin SDK/%s", o.TerraformVersion, meta.SDKVersionString()) //nolint:staticcheck

terraform-provider-azurestack/internal/common/client_options.go
62:	tfUserAgent := fmt.Sprintf("HashiCorp Terraform/%s (+https://www.terraform.io) Terraform Plugin SDK/%s", tfVersion, meta.SDKVersionString())

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a commit that fixes the action nevertheless – who knows what else is happening out there 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

I like it! I had not seen the existing var SDKPrerelease.

@ansgarm ansgarm force-pushed the prepare-pre-release-2.37.0-alpha.1 branch 3 times, most recently from 5b27eee to c9d373c Compare March 19, 2025 16:45
Comment on lines +92 to +93
sed -i "s/var SDKVersion =.*/var SDKVersion = \"${{ needs.changelog-version.outputs.version_only }}\"/" meta/meta.go
sed -i "s/var SDKPrerelease =.*/var SDKPrerelease = \"${{ needs.changelog-version.outputs.prerelease }}\"/" meta/meta.go
Copy link
Member

Choose a reason for hiding this comment

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

Honestly.... I'm wondering if we should just include the full version in SDKVersion and then the SDKPrerelease as the separated portion..

Because the SDKVersion shouldn't be v2.37.0 when we release this, it should really be v2.37.0-alpha.1. I don't think SDKPrerelease is super useful given that context, but since it exists, probably best to keep it up to date 😆

So when we release I think it'd make sense to be:

var SDKVersion = "v2.37.0-alpha.1"
var SDKPrerelease = "alpha.1"

Rather than:

var SDKVersion = "v2.37.0"
var SDKPrerelease = "alpha.1"

Copy link
Member

Choose a reason for hiding this comment

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

I guess maybe not because most of @bbasata's examples are using the SDKVersionString() instead, so it'd improperly format it then.... It's annoying these variables are exposed if we don't want people to reference them 😆

Copy link
Member

Choose a reason for hiding this comment

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

I guess also, all of this is deprecated anyways, so it doesn't really matter 🤷🏻

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay with prioritizing SDKVersionString() correctness. This is consistent with the doc comment for SDKVersion: The main version number that is being run at the moment.

@ansgarm ansgarm marked this pull request as ready for review March 20, 2025 08:12
@ansgarm ansgarm requested a review from a team as a code owner March 20, 2025 08:12
@ansgarm ansgarm force-pushed the prepare-pre-release-2.37.0-alpha.1 branch from 75bd7e3 to 9d44cf6 Compare March 20, 2025 09:23
@bbasata
Copy link
Contributor

bbasata commented Mar 20, 2025

💭 Future consideration: could we use go generate in the release workflow to write/update meta.go?

(OK, someone has been reading about go generate recently 😄)

@ansgarm
Copy link
Member Author

ansgarm commented Mar 20, 2025

could we use go generate

I'd love such a PR!

@ansgarm ansgarm merged commit c8df4e6 into main Mar 20, 2025
3 checks passed
@ansgarm ansgarm deleted the prepare-pre-release-2.37.0-alpha.1 branch March 20, 2025 14:19
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants