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

185 refractor init methods from main to initialize #186

Merged
merged 24 commits into from
Feb 13, 2024
Merged

185 refractor init methods from main to initialize #186

merged 24 commits into from
Feb 13, 2024

Conversation

cmatlak
Copy link
Contributor

@cmatlak cmatlak commented Feb 7, 2024 •
edited
Loading

Moved Auth methods to Auth class

cmatlak and others added 2 commits February 7, 2024 08:52
@cmatlak
Moved initilizeServer method to Initialize class
1fd0b83
@cmatlak @robinalfengard
Moved Auth methods to Auth class
f62a40e 
Co-authored-by: robinalfengard <robin.alfengard@hotmail.com>
@cmatlak cmatlak linked an issue Feb 7, 2024 that may be closed by this pull request
Refractor init methods from main to initialize #185
Closed
TrungMa1
TrungMa1 previously approved these changes Feb 7, 2024
View reviewed changes
Mattan41
Mattan41 requested changes Feb 8, 2024
View reviewed changes
Copy link
Contributor

@Mattan41 Mattan41 left a comment

Choose a reason for hiding this comment

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

methods seems to be refactored in orderly fashion, could we add some tests to increase code coverage for the methods that were moved to Auth and initialize?

@cmatlak
Copy link
Contributor Author

cmatlak commented Feb 8, 2024

thank you for the feedback, tests will be added. sometimes my trigger finger is faster than the brain

@Mattan41 Mattan41 self-assigned this Feb 9, 2024
Mattan41 and others added 9 commits February 9, 2024 12:50
Mattan41
Mattan41 previously approved these changes Feb 9, 2024
View reviewed changes
Copy link
Contributor

@Mattan41 Mattan41 left a comment

Choose a reason for hiding this comment

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

85.7% Coverage on New Code with the new tests

@Mattan41 Mattan41 dismissed their stale review February 9, 2024 17:59

I have assigned myself to the pull request

Danback
Danback previously approved these changes Feb 10, 2024
View reviewed changes
Copy link

@Danback Danback 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 to me, good job!

TrungMa1
TrungMa1 previously approved these changes Feb 11, 2024
View reviewed changes
Copy link
Contributor

@TrungMa1 TrungMa1 left a comment

Choose a reason for hiding this comment

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

Well done!

@Mattan41 Mattan41 mentioned this pull request Feb 12, 2024
Closed
@cmatlak cmatlak dismissed stale reviews from TrungMa1 and Danback via 058ce05 February 12, 2024 11:02
@cmatlak
Merge remote-tracking branch 'origin/185-refractor-init-methods-from-…
7c4d5e3 
…main-to-initialize' into 185-refractor-init-methods-from-main-to-initialize

# Conflicts:
#	src/test/java/org/fungover/haze/AuthTest.java
#	src/test/java/org/fungover/haze/MainTest.java
@cmatlak
deleted some stuff
e54f831
@cmatlak
deleted some stuff
2473e3d
@cmatlak
deleted some stuff
b1de023
@cmatlak
Copy link
Contributor Author

cmatlak commented Feb 12, 2024

this update should also solve remaining issues in #206 refactor-main i think

robinalfengard
robinalfengard previously approved these changes Feb 12, 2024
View reviewed changes
Copy link
Contributor

@robinalfengard robinalfengard left a comment

Choose a reason for hiding this comment

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

Should result in cleaner separation between Auth methods and Main, good job!

TrungMa1
TrungMa1 previously approved these changes Feb 13, 2024
View reviewed changes
Copy link
Contributor

@TrungMa1 TrungMa1 left a comment

Choose a reason for hiding this comment

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

Well done!

Danback
Danback previously approved these changes Feb 13, 2024
View reviewed changes
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Feb 13, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
80.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

robinalfengard
robinalfengard approved these changes Feb 13, 2024
View reviewed changes
Copy link
Contributor

@robinalfengard robinalfengard left a comment

Choose a reason for hiding this comment

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

Changes looks good!

@robinalfengard robinalfengard added this pull request to the merge queue Feb 13, 2024
Merged via the queue into main with commit d24d6a0 Feb 13, 2024
3 checks passed
@robinalfengard robinalfengard deleted the 185-refractor-init-methods-from-main-to-initialize branch February 13, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TrungMa1 TrungMa1 TrungMa1 approved these changes

@robinalfengard robinalfengard robinalfengard approved these changes

@Mattan41 Mattan41 Awaiting requested review from Mattan41

@Danback Danback Awaiting requested review from Danback

Assignees

@Mattan41 Mattan41

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refractor init methods from main to initialize
5 participants
@cmatlak @robinalfengard @Danback @Mattan41 @TrungMa1