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
johnstcn merged 6 commits intomainfrom
cj/ch15962/rbac_ssrr_fixes
Sep 13, 2021
Merged

address comments from #11#15
johnstcn merged 6 commits intomainfrom
cj/ch15962/rbac_ssrr_fixes

Conversation

@johnstcn
Copy link
Copy Markdown
Member

This PR addresses comments from #11

@shortcut-integration
Copy link
Copy Markdown

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
Copy Markdown

@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
test/go:
@echo "--- go test"
go test -parallel=$(shell nproc) ./...
$(shell scripts/test_go.sh)
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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