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

23 server port #71

Merged
merged 8 commits into from
Feb 15, 2023
Merged

23 server port #71

merged 8 commits into from
Feb 15, 2023

Conversation

safstromo
Copy link
Contributor

Code for setting the port number on startup

Priority:
CLI Options
Enviroment varable
Default port

@safstromo safstromo linked an issue Feb 9, 2023 that may be closed by this pull request
Server port #23
Closed
RobertMili
RobertMili previously approved these changes Feb 10, 2023
View reviewed changes
Copy link
Contributor

@RobertMili RobertMili left a comment

Choose a reason for hiding this comment

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

👍Looks great

Henrik-Bengtsson
Henrik-Bengtsson suggested changes Feb 13, 2023
View reviewed changes
Copy link
Contributor

@Henrik-Bengtsson Henrik-Bengtsson left a comment

Choose a reason for hiding this comment

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

Your test portSetByEnvironmentVariableShouldReturn6380 returns your default port 6379 so it fails.

Othervise it looks good

@safstromo
Copy link
Contributor Author

safstromo commented Feb 13, 2023 •
edited
Loading

Your test portSetByEnvironmentVariableShouldReturn6380 returns your default port 6379 so it fails.

Othervise it looks good

<plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-surefire-plugin</artifactId> <version>3.0.0-M7</version> <configuration> <environmentVariables> <HAZE_PORT>6380</HAZE_PORT> </environmentVariables> </configuration> </plugin>

Screenshot from 2023-02-13 12-03-00

needs to be at the bottom of the pom.xml for that test to work

@Henrik-Bengtsson
Copy link
Contributor

Henrik-Bengtsson commented Feb 13, 2023 •
edited
Loading

Had it added allready, but forget to refresh maven. So now it worked.

Henrik-Bengtsson
Henrik-Bengtsson previously approved these changes Feb 13, 2023
View reviewed changes
Copy link
Contributor

@Henrik-Bengtsson Henrik-Bengtsson 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.

@kappsegla kappsegla mentioned this pull request Feb 14, 2023
Closed
kappsegla
kappsegla requested changes Feb 14, 2023
View reviewed changes
Copy link
Contributor

@kappsegla kappsegla left a comment

Choose a reason for hiding this comment

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

Needs a rebase on latest version of main. But can be done after being put in the merge queue so you know what prior code to merge with.

src/main/java/org/fungover/haze/Initialize.java Outdated Show resolved Hide resolved
RobertMili
RobertMili previously approved these changes Feb 15, 2023
View reviewed changes
xCandyFun
xCandyFun previously approved these changes Feb 15, 2023
View reviewed changes
@safstromo safstromo dismissed stale reviews from xCandyFun and RobertMili via 21bb918 February 15, 2023 08:39
@sonarcloud
Copy link

sonarcloud bot commented Feb 15, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

79.3% 79.3% Coverage
0.0% 0.0% Duplication

@kappsegla kappsegla added this pull request to the merge queue Feb 15, 2023
Merged via the queue into main with commit 95d9e3e Feb 15, 2023
@kappsegla kappsegla added the enhancement New feature or request label Feb 15, 2023
@jLereback jLereback deleted the 23-server-port branch February 16, 2023 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xCandyFun xCandyFun xCandyFun approved these changes

@Lugn1 Lugn1 Lugn1 approved these changes

@kappsegla kappsegla kappsegla approved these changes

@RobertMili RobertMili Awaiting requested review from RobertMili

@Henrik-Bengtsson Henrik-Bengtsson Awaiting requested review from Henrik-Bengtsson

@wassimtajeddin wassimtajeddin Awaiting requested review from wassimtajeddin

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Server port
6 participants
@safstromo @Henrik-Bengtsson @kappsegla @RobertMili @Lugn1 @xCandyFun