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

[NET-3024] Configure GitHub Actions #544

Merged
merged 20 commits into from Mar 14, 2023
Merged

[NET-3024] Configure GitHub Actions #544

merged 20 commits into from Mar 14, 2023

Conversation

loshz
Copy link
Member

@loshz loshz commented Mar 8, 2023

Signed-off-by: Dan Bond danbond@protonmail.com

Description

This PR adds CI workflows for running Go tests in GitHub Actions. It uses the same commands as the existing CircleCI workflow and builds in both x86 and x86-64 architectures.

Other changes:

  • Add CODEOWNERS
  • Configure dependabot
  • go fmt ./...
  • Fix issues in tests related to go vet
  • Remove ./circleci config.

Note: we don't need to upload the test output xml as this was a CircleCI specific task https://github.com/hashicorp/raft/blob/main/.circleci/config.yml#L91-L94

loshz added 16 commits March 8, 2023 14:44
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
@loshz loshz marked this pull request as ready for review March 10, 2023 17:12
@loshz loshz requested review from modrake and shore March 10, 2023 17:13
.github/workflows/ci.yml Outdated Show resolved Hide resolved
Signed-off-by: Dan Bond <danbond@protonmail.com>
@loshz loshz requested a review from modrake March 10, 2023 17:39
go.mod Outdated
@@ -1,6 +1,6 @@
module github.com/hashicorp/raft

go 1.16
go 1.20
Copy link
Member

@banks banks Mar 10, 2023

Choose a reason for hiding this comment

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

Is this required? I don't think we want to force all downstream users of the library to only be able to build on the latest go release.

This number in go.mod is the "minimum supported version" not "the current version" and we should only update it if we are actually using a new feature of the language that requires at least this version.

If we made this change, we'd could have issues with our product builds if they are not yet ready for 1.20 or even have trouble pulling fixes into backports of our old releases that have to use older go versions etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not required, no.

Which Go versions do you recommend we build on in CI? Currently we do 1.16/1.17, but they're no longer supported.

Copy link
Member

Choose a reason for hiding this comment

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

I think last time we bumped it we decided "last two" on the basis that that covered most of our supported product releases. But IMO we should leave this as 1.16 until we actually have a specific reason to need language features that are worth breaking compatibility for.

Might be worth checking what Go versions last supported Consul/Nomad/Vault branches built from of you want to be sure but IMO risk is low for this library that we'd introduce backward incompatible changes given how little we commit here so last 2 versions is probably fine?

Copy link
Member Author

@loshz loshz Mar 13, 2023

Choose a reason for hiding this comment

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

Looks like Consul/Nomad are on 1.20 and Vault is on 1.19. I will continue building on latest 2 Go versions, but will revert the changes to go.mod even though it's "probably" safe.

Signed-off-by: Dan Bond <danbond@protonmail.com>
Signed-off-by: Dan Bond <danbond@protonmail.com>
@loshz loshz requested a review from banks March 13, 2023 16:32
Copy link
Member

@banks banks left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

Signed-off-by: Dan Bond <danbond@protonmail.com>
@loshz loshz merged commit 7719ff6 into main Mar 14, 2023
@loshz loshz deleted the loshz/NET-3024 branch March 14, 2023 18:29
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.

None yet

3 participants