Skip to content

Conversation

@pete0emerson
Copy link
Contributor

This closes #75

I validated each tag, discarding any that aren't SemVer.

For testing, I suggest that a non-SemVer tag be added to the public and private test repositories github_test.go.

fetch/github_test.go

Lines 25 to 28 in 4bec67f

{"https://github.com/gruntwork-io/fetch-test-public", "v0.0.1", "v0.0.3", "", testInst},
// Private repo equivalent
{"https://github.com/gruntwork-io/fetch-test-private", "v0.0.2", "v0.0.2", os.Getenv("GITHUB_OAUTH_TOKEN"), testInst},

@pete0emerson pete0emerson changed the title Pete/75/tags without semver Skip tags that are not Semantically Versioned Oct 22, 2020
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Could you add an automated test to validate this behavior?

@pete0emerson
Copy link
Contributor Author

Could you add an automated test to validate this behavior?

I've added expectedNumTags and set it to 3 for https://github.com/gruntwork-io/fetch-test-public and 1 for https://github.com/gruntwork-io/fetch-test-private (for which I don't have access, so I can't verify that 1 is correct, but I think it is since firstReleaseTag and lastReleaseTag are both set to v0.0.2).

To fully be confident this is always working, I'd suggest adding a couple of non-SemVer tags to both of those repositories; the expectedNumTags values I've set shouldn't change, but having a "bad" tag in there will then hit the code that skips those tags.

@pete0emerson pete0emerson requested a review from brikis98 October 26, 2020 20:53
@brikis98
Copy link
Member

I've added expectedNumTags and set it to 3 for https://github.com/gruntwork-io/fetch-test-public and 1 for https://github.com/gruntwork-io/fetch-test-private (for which I don't have access, so I can't verify that 1 is correct, but I think it is since firstReleaseTag and lastReleaseTag are both set to v0.0.2).

To fully be confident this is always working, I'd suggest adding a couple of non-SemVer tags to both of those repositories; the expectedNumTags values I've set shouldn't change, but having a "bad" tag in there will then hit the code that skips those tags.

Nice, thanks!

I added a non-semantic tag to both repos that looks like this: https://github.com/gruntwork-io/fetch-test-public/releases/tag/non-semantic

Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Code LGTM! Could you pull in the latest from master, and I'll kick off tests?

@pete0emerson
Copy link
Contributor Author

Done, and my local tests are passing until I hit the private repos.

Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Code LGTM! I'll kick off tests now.

@brikis98
Copy link
Member

Tests passed! Merging now.

@brikis98 brikis98 merged commit 8a6d52f into gruntwork-io:master Oct 28, 2020
@brikis98
Copy link
Member

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.

Handle tags that do not follow semantic versioning

2 participants