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

Fix usage of global opts in backplane session #159

Merged

Conversation

mrbarge
Copy link
Contributor

@mrbarge mrbarge commented Jul 19, 2023

What type of PR is this?

Bug

What this PR does / Why we need it?

PR #148 moved the --manager and --session flags to global options, but these do not get set in backplane session when used. This is because the session's internal options boolean variables are separate to those in the globalOpts struct, so they're never going to get set to what gets set in globalOpts.

The fix is straightforward, just add the global opts to session's own options struct as a pointer.

@openshift-ci openshift-ci bot requested review from bmeng and hectorakemp July 19, 2023 05:29
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 19, 2023

@mrbarge: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@codecov-commenter
Copy link

Codecov Report

Merging #159 (1e4194b) into main (318e781) will decrease coverage by 0.02%.
The diff coverage is 33.33%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
- Coverage   49.52%   49.50%   -0.02%     
==========================================
  Files          51       51              
  Lines        3352     3351       -1     
==========================================
- Hits         1660     1659       -1     
  Misses       1412     1412              
  Partials      280      280              
Impacted Files Coverage Δ
pkg/cli/session/session.go 50.44% <0.00%> (ø)
cmd/ocm-backplane/session/session.go 75.00% <100.00%> (-0.68%) ⬇️

@samanthajayasinghe
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2023
@feichashao
Copy link
Contributor

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: feichashao, mrbarge, samanthajayasinghe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 21, 2023
@openshift-merge-robot openshift-merge-robot merged commit 16bcc20 into openshift:main Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants