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

[KOGITO-2119] - Kogito CLI support for setting variable values from s… #404

Merged
merged 8 commits into from
Jul 16, 2020
Merged

[KOGITO-2119] - Kogito CLI support for setting variable values from s… #404

merged 8 commits into from
Jul 16, 2020

Conversation

vaibhavjainwiz
Copy link
Contributor

@vaibhavjainwiz vaibhavjainwiz commented Jun 25, 2020 •
edited

…ecret

Jira issue : https://issues.redhat.com/browse/KOGITO-2119

Many thanks for submiting your Pull Request ❤️!

Please make sure that your PR meets the following requirements:

  • You have read the contributors guide
  • Pull Request title is properly formatted: [KOGITO-XYZ] Subject
  • Pull Request contains link to the JIRA issue
  • Pull Request contains description of the issue
  • Pull Request does not include fixes for issues other than the main ticket
  • Your feature/bug fix has a unit test that verifies it
  • You've tested the new feature/bug fix in an actual OpenShift cluster

@vaibhavjainwiz vaibhavjainwiz added the needs review 🔍 Pull Request that needs reviewers label Jun 25, 2020
@ricardozanini ricardozanini removed the request for review from radtriste June 25, 2020 11:25
@ricardozanini ricardozanini added this to the v0.12.0 milestone Jun 25, 2020
ricardozanini
ricardozanini suggested changes Jun 25, 2020
View reviewed changes
Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

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

LGTM! One nitpick only. :)

cmd/kogito/command/shared/converters.go Outdated Show resolved Hide resolved
spolti
spolti approved these changes Jun 25, 2020
View reviewed changes
Copy link
Member

@spolti spolti left a comment

Choose a reason for hiding this comment

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

Approving. but before merge apply Zanini's suggestion.

@ricardozanini
Copy link
Member

We merged some CI changes, could you please rebase?

radtriste
radtriste reviewed Jun 29, 2020
View reviewed changes
Copy link
Contributor

@radtriste radtriste left a comment •
edited

Choose a reason for hiding this comment

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

@sutaakar @ricardozanini @spolti @Kaitou786 See also my comment

cmd/kogito/command/shared/converters.go Outdated Show resolved Hide resolved
@vaibhavjainwiz
Copy link
Contributor Author

We merged some CI changes, could you please rebase?

done

@ricardozanini ricardozanini modified the milestones: v0.12.0, v0.13.0 Jul 7, 2020
@codecov
Copy link

codecov bot commented Jul 10, 2020 •
edited

Codecov Report

Merging #404 into master will increase coverage by 0.20%.
The diff coverage is 65.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #404      +/-   ##
==========================================
+ Coverage   43.67%   43.88%   +0.20%     
==========================================
  Files         144      145       +1     
  Lines        8691     8725      +34     
==========================================
+ Hits         3796     3829      +33     
+ Misses       4469     4463       -6     
- Partials      426      433       +7
Flag Coverage Δ
#cli 68.29% <58.66%> (+0.20%) ⬆️
#operator 38.32% <100.00%> (+0.03%) ⬆️
Impacted Files Coverage Δ
pkg/util/maps.go 84.21% <ø> (+18.35%) ⬆️
cmd/kogito/command/deploy/common.go 52.00% <16.66%> (-1.85%) ⬇️
cmd/kogito/command/deploy/deploy_service.go 72.96% <33.33%> (-0.52%) ⬇️
cmd/kogito/command/install/data_index.go 82.92% <50.00%> (ø)
cmd/kogito/command/install/jobs_service.go 79.10% <50.00%> (ø)
cmd/kogito/command/install/mgmt_console.go 87.50% <50.00%> (ø)
cmd/kogito/command/util/common_util.go 61.90% <61.90%> (ø)
cmd/kogito/command/shared/converters.go 62.16% <76.66%> (+62.16%) ⬆️
pkg/framework/env_var.go 65.47% <100.00%> (+3.63%) ⬆️

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 573db7c...fd88f2c. Read the comment docs.

@ricardozanini ricardozanini self-requested a review July 10, 2020 10:18
ricardozanini
ricardozanini approved these changes Jul 13, 2020
View reviewed changes
Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

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

Looks good!

cmd/kogito/command/util/common_util_test.go Show resolved Hide resolved
cmd/kogito/command/shared/converters.go Show resolved Hide resolved
@radtriste
Copy link
Contributor

@vaibhavjainwiz can you confirm I can merge that one ?

@ricardozanini ricardozanini added ready 🚀 PR is ready to be merged on hold ✋ Pull Request is waiting for other dependencies and removed needs review 🔍 Pull Request that needs reviewers labels Jul 15, 2020
@ricardozanini
Copy link
Member

Adding onhold to wait for @vaibhavjainwiz confirmation.
@radtriste I'll send a PR to bump master to 0.13, please do not merge any PRs that impacts CRDs/CRs.

@vaibhavjainwiz
Copy link
Contributor Author

@vaibhavjainwiz can you confirm I can merge that one ?

@ricardozanini @radtriste Please merge this PR

@ricardozanini ricardozanini removed the on hold ✋ Pull Request is waiting for other dependencies label Jul 16, 2020
@ricardozanini ricardozanini merged commit 1240f7a into apache:master Jul 16, 2020
@vaibhavjainwiz vaibhavjainwiz deleted the KOGITO-2119 branch July 17, 2020 09:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sutaakar sutaakar sutaakar approved these changes

@ricardozanini ricardozanini ricardozanini approved these changes

@spolti spolti spolti approved these changes

@radtriste radtriste radtriste approved these changes

@Kaitou786 Kaitou786 Kaitou786 approved these changes

Assignees
No one assigned
Labels
ready 🚀 PR is ready to be merged
Projects
None yet
Milestone
v0.13.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@vaibhavjainwiz @ricardozanini @radtriste @spolti @sutaakar @Kaitou786