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

Update release fetching to optionally match tags with a platform component #32

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

samsymons
Copy link
Contributor

Task: https://app.asana.com/0/72649045549333/1209266752736135/f

This PR updates the GitHub release lookup logic to look through all releases in our repos and find the relevant release for the platform. It's implemented to keep working the way it always has for now, but when we're ready we would start passing the platform string into the lookup function and start getting back releases with a +platform string at the end.

How to test:

  1. Check that CI is green and that new test cases correctly test the behavior
  2. Verify that the changes don't break existed release logic
  3. Maybe do a proper test release in the test repo? I haven't done this yet, and have only relied on unit tests to verify the changes

@samsymons samsymons requested a review from kshann January 29, 2025 01:12
client = Octokit::Client.new(access_token: github_token)

current_page = 1
page_size = 25
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to change this to something else, but I feel that 25 should be enough to always find the latest public release for a given platform.

@@ -77,6 +77,31 @@ def self.assert_branch_has_changes(release_branch)

changed_files.any?
end

def self.latest_release(repo_name, prerelease, platform = nil, github_token)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll probably change the prerelease parameter name - what it's really doing it controlling whether the function only looks for a public release, or instead looks for the most recent available release regardless of internal/public status.

# If `prerelease` is true, return the latest release that matches the platform regardless of whether it's public.
# If `prerelease` is false, then ensure that the release is public.
matching_release = releases.find do |release|
(prerelease || !release.prerelease) && (platform.nil? || release.tag_name.end_with?("+#{platform}"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm assuming we're fine to check for release.prelease as a way to determine whether a tag is public or internal. It was either this or using a regex to check the tag format, and this feels simpler, providing that it's an assumption we can rely on.

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.

1 participant