Skip to content

Add RBAC CLI targets for goreleaser/makefile - #136

Merged
David Hyde (DABH) merged 9 commits into
masterfrom
add-rbac-targets
Mar 29, 2019
Merged

Add RBAC CLI targets for goreleaser/makefile#136
David Hyde (DABH) merged 9 commits into
masterfrom
add-rbac-targets

Conversation

@DABH

@DABH David Hyde (DABH) commented Mar 28, 2019

Copy link
Copy Markdown
Contributor

Builds now go to dist/ccloud or dist/confluent instead of just dist.

Definitely feels like duplicating stuff here, but goreleaser is limited in its configurability (e.g. can't have one project that outputs binaries to different directories, sigh).

There's no dist-confluent target since that isn't fleshed out yet.

@DABH
David Hyde (DABH) requested a review from a team March 28, 2019 00:59
@DABH

Copy link
Copy Markdown
Contributor Author

(If current state gets approved I'll add some dummy rbac command etc. so semaphore passes)

@norwood

Copy link
Copy Markdown
Contributor

seems reasonable to me

@codecov

codecov Bot commented Mar 28, 2019

Copy link
Copy Markdown

Codecov Report

Merging #136 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #136      +/-   ##
=========================================
- Coverage   35.14%   35.1%   -0.04%     
=========================================
  Files          30      30              
  Lines        1781    1783       +2     
=========================================
  Hits          626     626              
- Misses       1087    1089       +2     
  Partials       68      68
Impacted Files Coverage Δ
internal/cmd/command.go 0% <0%> (ø) ⬆️
cmd/confluent/main.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1ad8dc...4e2a643. Read the comment docs.

Comment thread internal/cmd/command.go Outdated

@codyaray Cody A. Ray (codyaray) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

since we're building/shipping two binaries for CP 5.3 next release, this approach makes sense

Comment thread README.md Outdated
Comment thread cmd/confluent/main.go Outdated
Comment thread cmd/confluent/main_test.go Outdated

@norwood dan norwood (norwood) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sherp

Comment thread .goreleaser.yml
# TODO: [CLI-92] we delete the semaphore cache during release to workaround an issue with semaphore and goreleaser
- rm -rf $GOPATH/pkg/mod

# NOTE: This will put all builds into the same ./dist folder. There is no way to configure goreleaser output directories per-build, only per-project.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is there a TODO here or is this already accounted for in the makefile?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was told CI is using the .goreleaser.yml file while locally I know we're building with the -mac.yml or -linux.yml files. So, CI just throws everything in ./dist, whereas local builds shove stuff into ./dist/ccloud or ./dist/confluent. But I think that's fine since CI isn't pushing to s3, we're doing that manually with make targets (which use subfolders correctly)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

goreleaser will be pushing binaries to s3... but yeah, we push the archives ourselves from the Makefile.

The other approach is to have two separate goreleaser-CI files (one per binary). Then we can disable S3 altogether for confluent.

But I think we want binaries in S3 for both of them to support self-update. (I'm going to start saying "self-update" instead of auto-update... since we give users the choice. And maybe this won't be as scary sounding to management/product folks here. :)

Comment thread internal/cmd/command.go Outdated

@codyaray Cody A. Ray (codyaray) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

we can do more packaging stuff as a follow-up

@DABH
David Hyde (DABH) merged commit d806c5d into master Mar 29, 2019
@DABH
David Hyde (DABH) deleted the add-rbac-targets branch March 29, 2019 20:26
Brian Strauch (brianstrauch) pushed a commit that referenced this pull request May 31, 2023
…#136)

* add current user to the statement property as statement-ownder'

* recover .go-version

* recovered order of imports

* simplified 'if'
airlock-confluentinc Bot pushed a commit that referenced this pull request Sep 4, 2024
* chore: stop swallowing error when LSP client fails to be initialized (#136)

* flink statement resume

* flink statement update

* fix tests

* list compute pool flag first

* updated with revised plan

* fix tests

* rebase

* rebase test working

* Revert Last change

* Formatiing

* Failing test case

* Test Cases fix

* Lint

* Lint test case

---------

Co-authored-by: Yannick Pferr <37211050+YannickPferr@users.noreply.github.com>
Co-authored-by: Brian Strauch <bstrauch@confluent.io>
Co-authored-by: Tushar Malik <tmalik@confluent.io>
airlock-confluentinc Bot pushed a commit that referenced this pull request Sep 18, 2024
* chore: stop swallowing error when LSP client fails to be initialized (#136)

* flink statement resume

* flink statement update

* fix tests

* list compute pool flag first

* updated with revised plan

* fix tests

* rebase

* rebase test working

* Revert Last change

* Formatiing

* Failing test case

* Test Cases fix

* Lint

* Lint test case

* review

---------

Co-authored-by: Yannick Pferr <37211050+YannickPferr@users.noreply.github.com>
Co-authored-by: Brian Strauch <bstrauch@confluent.io>
Co-authored-by: Tushar Malik <tmalik@confluent.io>
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.

3 participants