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

feat: availableport enhancements #3621

Merged
merged 24 commits into from
Oct 13, 2023
Merged

Conversation

gfant
Copy link
Contributor

@gfant gfant commented Aug 9, 2023

Currently availableport.Find only allows the user to ask for ports between 44000 and 55000 and there is no verification about if the ports returned are the same.
With this pull request is fixed the last part and now the users can add through a struct different ranges of ports and a custom randomizer.
Also I added the tests for the editions made.

@gfant gfant changed the title fix: availableport enhancements feat: availableport enhancements Aug 9, 2023
@github-actions github-actions bot added component:ci CI/CD workflow and automated jobs. component:configs component:packages labels Aug 9, 2023
@gfant gfant changed the title feat: availableport enhancements feat: availableport enhancements Aug 9, 2023
Copy link
Collaborator

@Pantani Pantani 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 the contribution!!!

ignite/pkg/availableport/availableport.go Outdated Show resolved Hide resolved
ignite/pkg/availableport/availableport.go Outdated Show resolved Hide resolved
@Pantani
Copy link
Collaborator

Pantani commented Aug 10, 2023

can you please also add a changelog?

@Pantani
Copy link
Collaborator

Pantani commented Aug 30, 2023

@iam-agf any update on this PR?

@gfant
Copy link
Contributor Author

gfant commented Sep 3, 2023

Sorry for the super slow update. Many things happening, but I think now is ready!

@Pantani
Copy link
Collaborator

Pantani commented Oct 13, 2023

@iam-agf any updates? can you use the table tests in the test?

@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Merging #3621 (bf9f50c) into main (a86dd33) will increase coverage by 0.12%.
The diff coverage is 80.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3621      +/-   ##
==========================================
+ Coverage   24.99%   25.11%   +0.12%     
==========================================
  Files         286      287       +1     
  Lines       23489    23539      +50     
==========================================
+ Hits         5871     5912      +41     
- Misses      17090    17096       +6     
- Partials      528      531       +3     
Files Coverage Δ
ignite/config/chain/base/config.go 100.00% <ø> (ø)
ignite/config/chain/config.go 12.90% <0.00%> (ø)
ignite/pkg/availableport/availableport.go 82.00% <81.63%> (ø)

ignite/pkg/availableport/availableport.go Outdated Show resolved Hide resolved
ignite/pkg/availableport/availableport.go Outdated Show resolved Hide resolved
ignite/pkg/availableport/availableport_test.go Outdated Show resolved Hide resolved
@Pantani Pantani self-requested a review October 13, 2023 00:36
Pantani
Pantani previously approved these changes Oct 13, 2023
@Pantani Pantani self-assigned this Oct 13, 2023
jeronimoalbi
jeronimoalbi previously approved these changes Oct 13, 2023
changelog.md Outdated Show resolved Hide resolved
@jeronimoalbi jeronimoalbi dismissed stale reviews from Pantani and themself via 1cc6d4c October 13, 2023 07:57
@jeronimoalbi jeronimoalbi merged commit 3c9f058 into ignite:main Oct 13, 2023
7 of 23 checks passed
@gfant
Copy link
Contributor Author

gfant commented Oct 14, 2023

@iam-agf any updates? can you use the table tests in the test?

Sorry for the late response. An apologize. I'll try to make it faster and correct next time.

@jeronimoalbi
Copy link
Member

jeronimoalbi commented Oct 16, 2023

@iam-agf any updates? can you use the table tests in the test?

Sorry for the late response. An apologize. I'll try to make it faster and correct next time.

No worries! Thank you for the contribution 🚀

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

Successfully merging this pull request may close these issues.

3 participants