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

Introduce visibility flag for bootstrap gitlab #4866

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Introduce visibility flag for bootstrap gitlab #4866

merged 1 commit into from
Jul 18, 2024

Conversation

nagyv
Copy link
Contributor

@nagyv nagyv commented Jun 28, 2024

This MR introduces a ––visibility=private|public|internal flag for bootstrap gitlab.

At the same time it deprecates the --private flag.

Related to #3817

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

@nagyv please upload your signing key to GitHub and also signoff your commits.

cmd/flux/bootstrap_gitlab.go Outdated Show resolved Hide resolved
@stefanprodan stefanprodan added the area/bootstrap Bootstrap related issues and pull requests label Jun 29, 2024
internal/flags/gitlab_visibility.go Show resolved Hide resolved
internal/flags/gitlab_visibility_test.go Outdated Show resolved Hide resolved
internal/flags/gitlab_visibility.go Outdated Show resolved Hide resolved
cmd/flux/bootstrap_gitlab.go Outdated Show resolved Hide resolved
cmd/flux/bootstrap_gitlab.go Outdated Show resolved Hide resolved
cmd/flux/bootstrap_gitlab.go Outdated Show resolved Hide resolved
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

The current implementation looks good to me.
Left a few more minor comments and a question before we consider this ready.

Also, @nagyv some commits are missing git signoff due to which the CI checks are failing. I think you can squash the commits as appropriate and signoff only the relevant commits.

cmd/flux/bootstrap_gitlab.go Show resolved Hide resolved
internal/flags/gitlab_visibility.go Outdated Show resolved Hide resolved
internal/flags/gitlab_visibility.go Outdated Show resolved Hide resolved
@nagyv
Copy link
Contributor Author

nagyv commented Jul 15, 2024 via email

Copy link
Contributor

@darkowlzz darkowlzz 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.

@darkowlzz
Copy link
Contributor

Are you ok approving?

@nagyv I have approved it. This branch is out-of-date with the base branch.
Can you please rebase against the latest main branch and squash the commits to a single or a few relevant commits?

@darkowlzz darkowlzz requested a review from a team July 15, 2024 13:27
Copy link
Member

@stefanprodan stefanprodan 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 @nagyv 🏅

PS. Please squash all commits into one and rebase with upstream main.

@nagyv
Copy link
Contributor Author

nagyv commented Jul 17, 2024

@darkowlzz @stefanprodan Squashed and rebased. 🚀

@stefanprodan
Copy link
Member

You need to rebase with upstream, your fork is behind

@nagyv
Copy link
Contributor Author

nagyv commented Jul 17, 2024

@stefanprodan Are we good now?

@stefanprodan stefanprodan merged commit 31d160b into fluxcd:main Jul 18, 2024
7 checks passed
@stefanprodan stefanprodan mentioned this pull request Aug 22, 2024
57 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bootstrap Bootstrap related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants