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

Filter Release Tags #46

Merged
merged 5 commits into from
Nov 2, 2023
Merged

Filter Release Tags #46

merged 5 commits into from
Nov 2, 2023

Conversation

afujiwara-roblox
Copy link
Contributor

The current behavior is for setup-foreman to fail if it encounters an invalid semver tag in the Foreman repo. This PR looks to
add a filter step to filter out invalid semver tags.

Copy link
Contributor

@ZoteTheMighty ZoteTheMighty left a comment

Choose a reason for hiding this comment

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

Looks good, just one suggestion for the sake of regression testing

{
tag_name: "4.3.0",
assets: []
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for good measure, can we add one that starts with v but isn't valid? 😀

function filterValidReleases(releases: GitHubRelease[]): GitHubRelease[] {
return releases.filter(release => {
const tag = release.tag_name;
return tag.startsWith("v") && semver.valid(tag);
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that semver.valid must handle the leading v just fine, since your tests appear to pass

@afujiwara-roblox afujiwara-roblox merged commit fca1151 into master Nov 2, 2023
9 checks passed
@afujiwara-roblox afujiwara-roblox deleted the filter-release-tags branch November 2, 2023 20:45
@github-actions github-actions bot locked and limited conversation to collaborators Nov 2, 2023
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.

2 participants