Skip to content
This repository was archived by the owner on Nov 14, 2024. It is now read-only.

address comments from #11 #15

Merged
merged 6 commits into from
Sep 13, 2021
Merged

address comments from #11 #15

merged 6 commits into from
Sep 13, 2021

Conversation

johnstcn
Copy link
Member

This PR addresses comments from #11

@shortcut-integration
Copy link

This pull request has been linked to Clubhouse Story #15962: Checks for required cluster API resources.

@johnstcn johnstcn requested a review from jawnsy September 13, 2021 10:29
@johnstcn johnstcn self-assigned this Sep 13, 2021
Copy link
Contributor

@jawnsy jawnsy left a comment

Choose a reason for hiding this comment

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

LGTM

Makefile Outdated
@@ -33,5 +33,5 @@ test: test/go

test/go:
@echo "--- go test"
go test -parallel=$(shell nproc) ./...
$(shell scripts/test_go.sh)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can just do ./scripts/test_go.sh here, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, yes that's correct!

run_trace false goveralls -service=github -coverprofile="$COVERAGE"
set -e
# Skip coveralls if not running in CI
if [[ -n "${CI}" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

ah, yeah. We don't use this script to run local tests in the monorepo, which is why we didn't need that check there 🤷‍♂️

@johnstcn johnstcn merged commit fc1b142 into main Sep 13, 2021
@johnstcn johnstcn deleted the cj/ch15962/rbac_ssrr_fixes branch September 13, 2021 14:33
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