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.

Sign up for GitHub

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

Jump to bottom

SANDBOX-674: add NewBannedUser and IsAlreadyBanned funcs #413

Merged
merged 11 commits into from
Jul 18, 2024
Merged

SANDBOX-674: add NewBannedUser and IsAlreadyBanned funcs #413

merged 11 commits into from
Jul 18, 2024

Conversation

rsoaresd
Copy link
Contributor

@rsoaresd rsoaresd commented Jul 15, 2024 •
edited
Loading

Description

  • add NewBannedUser and IsAlreadyBanned funcs to avoid duplicate this work across the repos

Issue ticket number and link

SANDBOX-674

Associated PRs:

@codecov Codecov
Copy link

codecov bot commented Jul 15, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.45%. Comparing base (8604fe4) to head (abe74d4).

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #413      +/-   ##
==========================================
+ Coverage   78.15%   78.45%   +0.30%     
==========================================
  Files          47       48       +1     
  Lines        1982     2010      +28     
==========================================
+ Hits         1549     1577      +28     
  Misses        375      375              
  Partials       58       58
Files Coverage Δ
pkg/banneduser/banneduser.go 100.00% <100.00%> (ø)

alexeykazakov
alexeykazakov reviewed Jul 16, 2024
View reviewed changes
Copy link
Collaborator

@alexeykazakov alexeykazakov left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good! But there is some room for code coverage improvements :) See the codecov report.

pkg/banneduser/banneduser.go Outdated Show resolved Hide resolved
alexeykazakov
alexeykazakov reviewed Jul 16, 2024
View reviewed changes
pkg/banneduser/banneduser.go Outdated Show resolved Hide resolved
pkg/banneduser/banneduser.go Outdated Show resolved Hide resolved
alexeykazakov
alexeykazakov reviewed Jul 16, 2024
View reviewed changes
Copy link
Collaborator

@alexeykazakov alexeykazakov left a comment

Choose a reason for hiding this comment

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

Nice! Just a few minor comments.

pkg/banneduser/banneduser.go Outdated Show resolved Hide resolved
pkg/banneduser/banneduser.go Outdated Show resolved Hide resolved
pkg/banneduser/banneduser_test.go Outdated Show resolved Hide resolved
pkg/banneduser/banneduser_test.go Outdated Show resolved Hide resolved
pkg/banneduser/banneduser_test.go Show resolved Hide resolved
rajivnathan
rajivnathan reviewed Jul 16, 2024
View reviewed changes
Copy link
Contributor

@rajivnathan rajivnathan left a comment

Choose a reason for hiding this comment

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

Nice, just a couple minor comments

pkg/banneduser/banneduser_test.go Outdated Show resolved Hide resolved
pkg/banneduser/banneduser_test.go Outdated Show resolved Hide resolved
fbm3307
fbm3307 reviewed Jul 17, 2024
View reviewed changes
Copy link
Contributor

@fbm3307 fbm3307 left a comment

Choose a reason for hiding this comment

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

Just some minor suggestions

pkg/banneduser/banneduser.go Outdated Show resolved Hide resolved
pkg/banneduser/banneduser.go Outdated Show resolved Hide resolved
@rsoaresd rsoaresd requested a review from fbm3307 July 17, 2024 11:37
alexeykazakov
alexeykazakov approved these changes Jul 17, 2024
View reviewed changes
Copy link
Collaborator

@alexeykazakov alexeykazakov left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comments!

@rsoaresd rsoaresd merged commit c8ee880 into codeready-toolchain:master Jul 18, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexeykazakov alexeykazakov alexeykazakov approved these changes

@MatousJobanek MatousJobanek Awaiting requested review from MatousJobanek MatousJobanek is a code owner

@xcoulon xcoulon Awaiting requested review from xcoulon xcoulon is a code owner

@ranakan19 ranakan19 Awaiting requested review from ranakan19 ranakan19 is a code owner

@sbryzak sbryzak Awaiting requested review from sbryzak

@mfrancisc mfrancisc Awaiting requested review from mfrancisc mfrancisc is a code owner

@rajivnathan rajivnathan Awaiting requested review from rajivnathan rajivnathan is a code owner

@fbm3307 fbm3307 Awaiting requested review from fbm3307

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rsoaresd @alexeykazakov @rajivnathan @fbm3307