Skip to content

Fork helm-operator adjustments for CI and documentation - #4

Merged
SimonBaeumer merged 3 commits into
mainfrom
add-ci
May 18, 2021
Merged

Fork helm-operator adjustments for CI and documentation#4
SimonBaeumer merged 3 commits into
mainfrom
add-ci

Conversation

@SimonBaeumer

@SimonBaeumerSimonBaeumer commented May 18, 2021

Copy link
Copy Markdown
Contributor

CI migration of the helm-operator fork.
Did not deleted the original deploy actions.

  • Disable some docker image push
  • Execute CI on all pushes

@SimonBaeumerSimonBaeumer changed the title Add CIAdjust CI to fork, disable image pushMay 18, 2021
@SimonBaeumerSimonBaeumer changed the title Adjust CI to fork, disable image pushFork helm-operator adjustments for CI and documentationMay 18, 2021
Comment threadREADME.md Outdated

Experimental refactoring of the operator-framework's helm operator

### Why a fork?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@gaurav-nelson Do you have any feedback for me here?
I would like to say that this is a fork which diverges in its use-case from the original operator implementation.

@msugakovmsugakov left a comment

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 guess it would be good to have some convention how to label commits for easier upstreaming our changes. For example, this commit must not be picked for upstreaming and can be marked somehow like [not for upstream] Fork helm-operator adjustments for CI and documentation.
What do you think?

- '**'
pull_request:
branches: [ main ]
- push

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.

Curious as to why you changed this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Because the CI was not triggered in the default setting.

Comment threadREADME.md Outdated
As the helm-operator is an experimental refactoring and not actively maintained we started a fork to
further support [hybrid operators](https://github.com/operator-framework/operator-sdk/issues/670) based on Helm.

This fork should used as a library and not is recommended to watch CustomResources by configured `watches`.

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.

I don't understand this sentence, or how "watching CustomResources by configured watches" relates to whether or not it's being used as a library

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Rephrased the whole paragraph.

Comment threadREADME.md Outdated
Add this lib as a replace directive to your `go.mod`:

```
replace(

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.

super nit: it's common to just provide a CLI command, e.g., go mod edit -replace=github.com/joelanford/helm-operator=github.com/stackrox/helm-operator@main (also, the line as it's written is invalid, as you need to specify a revision for the replacement module)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

done

@SimonBaeumer

Copy link
Copy Markdown
ContributorAuthor

I guess it would be good to have some convention how to label commits for easier upstreaming our changes. For example, this commit must not be picked for upstreaming and can be marked somehow like [not for upstream] Fork helm-operator adjustments for CI and documentation.
What do you think?

👍
Added labels for it, by default upstream-triage is added to indicate that it should be decided if this should be an upstream fix too.

@SimonBaeumer
SimonBaeumer merged commit a116e55 into mainMay 18, 2021
@vladbologavladbologa mentioned this pull request Aug 31, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@SimonBaeumer@msugakov@misberner