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

Add CI for Go #77

Merged
merged 17 commits into from
Feb 12, 2024
Merged

Add CI for Go #77

merged 17 commits into from
Feb 12, 2024

Conversation

aaron-congo
Copy link

@aaron-congo aaron-congo commented Feb 8, 2024

The CI workflow includes:

  • building the client
  • running unit tests
  • go vet and staticcheck as Go linters
  • the lint-rust workflow

@Yury-Fridlyand
Copy link

Does CI include linter?

@aaron-congo
Copy link
Author

aaron-congo commented Feb 8, 2024

Does CI include linter?

Yep, I added go vet and staticcheck as linters, and I also added the lint-rust step

go/Cargo.toml Show resolved Hide resolved
use std::ffi::c_void;

#[no_mangle]
pub extern "C" fn create_connection() -> *const c_void {
Copy link

Choose a reason for hiding this comment

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

I needed to add some arbitrary code here so that lint-rust had something to run against. I tried only adding the copyright, with no actual code, but the lint-rust workflow failed, I think because it wants their to be logic in this file

)

// TODO: Replace this test with real tests when glide client implementation is started
func TestArbitraryLogic(t *testing.T) {
Copy link

Choose a reason for hiding this comment

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

I also added this arbitrary test so that the unit test workflow had something to run against

package main

import (
_ "github.com/DarkDrim/go-test-report"
Copy link

Choose a reason for hiding this comment

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

Apparently this file is recommended to specify ci dependencies (ie dependencies that are not directly required by your code). These dependencies are specified here and in go.mod. If you remove this file the dependencies in go.mod are removed when you run go mod tidy because go detects that they aren't directly required by your code. More info about this here

@aaron-congo aaron-congo changed the title [WIP] Go CI Add CI for Go Feb 9, 2024
@aaron-congo aaron-congo closed this Feb 9, 2024
@aaron-congo aaron-congo deleted the go/dev_acongo_config branch February 9, 2024 23:33
@aaron-congo aaron-congo restored the go/dev_acongo_config branch February 9, 2024 23:33
@aaron-congo aaron-congo reopened this Feb 9, 2024
@aaron-congo
Copy link
Author

aaron-congo commented Feb 10, 2024

Couple related questions:

  • There's a few dependencies that are required to build the client, create a test report, and lint the Go code. What is the criteria for assessing whether they are okay to use with regard to licensing?
  • One recommendation with the Go project layout is to include an internal directory at the root of the Go project that contains private library code. Code in this directory cannot be imported by packages outside the source subtree in which they reside. For the public code that is meant to be used by others, we can either have it at the top level in the go directory or put it in a pkg directory. If we put it in pkg the top-level will not mix Go files and project files like .gitignore, Cargo.toml etc, but the downside is that users will need to include "pkg" in the module import path. Go developers seem to be pretty divided on which way is preferred. More details here

.github/workflows/go.yml Outdated Show resolved Hide resolved
ln -s /usr/bin/redis6-server /usr/bin/redis-server
ln -s /usr/bin/redis6-cli /usr/bin/redis-cli

- name: Install Go

Choose a reason for hiding this comment

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

Can't you use actions/setup-go there?

Copy link
Author

Choose a reason for hiding this comment

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

Maybe? I saw that Java uses setup-java for ubuntu/macos but not for amazon linux so I assumed setup-java did not work on amazon linux and that if it doesn't work for java that it wouldn't for go. But my assumption may be wrong

go/Makefile Show resolved Hide resolved
.github/workflows/go.yml Outdated Show resolved Hide resolved
go/go.mod Show resolved Hide resolved
.github/workflows/go.yml Show resolved Hide resolved
.github/workflows/go.yml Outdated Show resolved Hide resolved
@aaron-congo aaron-congo merged commit a2916a6 into go/integ_acongo_config Feb 12, 2024
10 checks passed
@aaron-congo aaron-congo deleted the go/dev_acongo_config branch February 12, 2024 22:32
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.

5 participants