Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

Automated chart bump jaeger-operator-2.18.4 #835

Merged
merged 3 commits into from
Feb 4, 2021

Conversation

d2iq-dispatch
Copy link
Contributor

@d2iq-dispatch d2iq-dispatch commented Jan 29, 2021

What type of PR is this?
Chore

What this PR does/ why we need it:
Bump jaeger.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

bump jaeger-operator-2.18.4

Checklist

  • If a chart is changed, the chart version is correctly incremented.
  • The commit message explains the changes and why are needed.
  • The code builds and passes lint/style checks locally.
  • The relevant subset of integration tests pass locally.
  • The core changes are covered by tests.
  • The documentation is updated where needed.

@d2iq-dispatch d2iq-dispatch requested review from a team as code owners January 29, 2021 03:01
@d2iq-dispatch d2iq-dispatch added help wanted Extra attention is needed needs release notes This PR needs the PR comment release-note block labels Jan 29, 2021
addons/jaeger/jaeger.yaml Outdated Show resolved Hide resolved
@joejulian
Copy link
Contributor

@GoelDeepak I assume you just want to close this?

@GoelDeepak GoelDeepak closed this Feb 3, 2021
@vespian
Copy link
Contributor

vespian commented Feb 3, 2021

I assume you just want to close this?

We just need to manually fix this. It was in my queue, but during the release time it was of low priority.

Fixing it now.

@vespian vespian reopened this Feb 3, 2021
@vespian vespian force-pushed the bump-jaeger-operator-3ee27cc5 branch from b8b949f to 0bb6aa5 Compare February 3, 2021 13:06
@vespian
Copy link
Contributor

vespian commented Feb 3, 2021

@mesosphere/sig-ksphere-catalog Can I have a review?

@vespian vespian enabled auto-merge (squash) February 3, 2021 16:31
@vespian vespian disabled auto-merge February 3, 2021 16:32
@vespian vespian added the ready label Feb 3, 2021
@vespian
Copy link
Contributor

vespian commented Feb 3, 2021

@joejulian @orsenthil Do you know what is the Mergable check? It does not allow me to merge PR, and does not clarify which label is missing either :(

@joejulian
Copy link
Contributor

@vespian If you go in to Details and look at Mergeable, there's this.
image
We started a really short discussion about making Mergeable add comments to state the same that can be found there. Would that be a desired behavior to you?

@joejulian
Copy link
Contributor

If you were an administrator for a production cluster and you wanted to know whether or not to upgrade your addons to this version, is this a valuable release note for you?

Personally, I would prefer to see what features or bug fixes that bump brings so I know whether it's something that helps me. That's what I look for in release notes, fwiw.

@joejulian
Copy link
Contributor

As an example:
https://github.com/mesosphere/kubernetes-base-addons/blob/master/RELEASE_NOTES.md#stable-118-300

I think cert-manager is a little bit too verbose, listing items that are trivial and don't apply to users. Those things that do, like "v1 API" don't have enough detail, like "This is a complete replacement of the v1alpha1 CRDs. This upgrade will change all necessary resources to use the new API. This upgrade cannot be reverted."

Dex is good. If I wasn't even noticing a lack of metrics from dex, this would alert me to it and make me want to upgrade.

Istio is way too verbose. It's effectively a copy-paste of upstream's auto-generated release notes that include a lot of trivial notes that have no user-facing impact.

@vespian
Copy link
Contributor

vespian commented Feb 4, 2021

We started a really short discussion about making Mergeable add comments to state the same that can be found there. Would that be a desired behavior to you?

No, the way it is now is way clearer. I am just not used to the Details link having useful data in PR status checks.

@vespian vespian removed the needs release notes This PR needs the PR comment release-note block label Feb 4, 2021
@vespian
Copy link
Contributor

vespian commented Feb 4, 2021

If you were an administrator for a production cluster and you wanted to know whether or not to upgrade your addons to this version, is this a valuable release note for you?

Depends on how much resources we have at our disposal.

Ideally yes - as an admin, I would appreciate my vendor to summarise the changes for me in just one place. In our current situation though I am struggling to find time to keep the bumps going and do not let #sig-networking slip into abandonment. Networking stuff it is not a priority ATM.

Thanks for understanding.

@vespian vespian enabled auto-merge (squash) February 4, 2021 08:19
@vespian vespian disabled auto-merge February 4, 2021 08:40
@armandgrillet armandgrillet merged commit 503b064 into master Feb 4, 2021
@armandgrillet armandgrillet deleted the bump-jaeger-operator-3ee27cc5 branch February 4, 2021 08:41
@joejulian joejulian removed the help wanted Extra attention is needed label Feb 11, 2021
@joejulian
Copy link
Contributor

@mesosphere-mergebot backport release/3.3

@d2iq-mergebot
Copy link
Contributor

Backport PR for release/3.3: #918

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants