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: add FixedLength#parse(Reader) #37

Merged
merged 1 commit into from
May 16, 2024
Merged

feat: add FixedLength#parse(Reader) #37

merged 1 commit into from
May 16, 2024

Conversation

mbechto
Copy link
Contributor

@mbechto mbechto commented May 15, 2024

Addresses #36. Please let me know if this is acceptable for you or if I should change anything.

@g0ddest
Copy link
Owner

g0ddest commented May 16, 2024

Hey, @mbechto ! Great job on this PR! The addition of the parse(Reader reader) method significantly enhances the library’s flexibility, allowing users to parse input from a wider range of sources. The implementation is clear and concise, adhering to the existing codebase patterns. I also appreciate the thorough unit tests, which ensure that the new functionality works as intended. This is a valuable and well-executed improvement.
One minor note: there is an unused import in the code that should be removed. Thanks for your contribution!

@mbechto
Copy link
Contributor Author

mbechto commented May 16, 2024

Right, got it removed 😃

@g0ddest
Copy link
Owner

g0ddest commented May 16, 2024

and squash commits please

@g0ddest g0ddest self-assigned this May 16, 2024
@g0ddest g0ddest added the good first issue Good for newcomers label May 16, 2024
src/test/java/ParserTest.java Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented May 16, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@g0ddest g0ddest merged commit 32e2522 into g0ddest:master May 16, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants