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

update projectOptions to be public by renaming it to ProjectOptions #10103

Merged
merged 1 commit into from Dec 20, 2022

Conversation

tigerinus
Copy link
Contributor

Signed-off-by: Tiger Wang tigerwang@outlook.com

What I did

  • updated projectOptions to be public by renaming it to ProjectOptions
  • updated toProject(...) method of projectOptions to be public by renaming it to ToProject(...)
  • tested all impacted code to make sure they are passing

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

my cat

This is my cat - she never shows up and always hide herself somewhere in the house.

Signed-off-by: Tiger Wang <tigerwang@outlook.com>
Copy link
Contributor

@ndeloof ndeloof left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Dec 19, 2022

Codecov Report

Base: 76.98% // Head: 75.79% // Decreases project coverage by -1.19% ⚠️

Coverage data is based on head (28b3776) compared to base (9d12eec).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10103      +/-   ##
==========================================
- Coverage   76.98%   75.79%   -1.20%     
==========================================
  Files           2        2              
  Lines         252      252              
==========================================
- Hits          194      191       -3     
- Misses         51       53       +2     
- Partials        7        8       +1     
Impacted Files Coverage Δ
pkg/e2e/framework.go 74.04% <0.00%> (-1.28%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tigerinus
Copy link
Contributor Author

tigerinus commented Dec 19, 2022

@ndeloof - how should I handle this code coverage issue? The change is literally turning lower cases to upper cases.

Should I add unit tests for ToProject method that is now public?

Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM

@ndeloof
Copy link
Contributor

ndeloof commented Dec 20, 2022

I also had strange results from codecov, seems this one is buggy :P

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

Successfully merging this pull request may close these issues.

None yet

3 participants